diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 5630964ad..2d1417126 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -394,12 +394,12 @@ func main() { // Initialize commitments API for LIQUID interface (Postgres-backed usage reporting). commitmentsConfig := conf.GetConfigOrDie[commitments.Config]() - var commitmentsUsageDB commitments.UsageDBClient + var commitmentsVMSource reservations.VMSource if commitmentsConfig.DatasourceName != "" { - commitmentsUsageDB = commitments.NewDBUsageClient(multiclusterClient, commitmentsConfig.DatasourceName) + commitmentsVMSource = reservations.NewPostgresVMSource(multiclusterClient, commitmentsConfig.DatasourceName) } if slices.Contains(mainConfig.EnabledControllers, "committed-resource-reservations-controller") { - commitmentsAPI := commitmentsapi.NewAPIWithConfig(multiclusterClient, commitmentsConfig.API, commitmentsUsageDB) + commitmentsAPI := commitmentsapi.NewAPIWithConfig(multiclusterClient, commitmentsConfig.API, commitmentsVMSource) commitmentsAPI.Init(mux, metrics.Registry, ctrl.Log.WithName("commitments-api")) } @@ -597,16 +597,16 @@ func main() { usageReconcilerMonitor := commitments.NewUsageReconcilerMonitor() metrics.Registry.MustRegister(&usageReconcilerMonitor) - if commitmentsUsageDB == nil { + if commitmentsVMSource == nil { setupLog.Error(nil, "UsageReconciler requires a datasource but commitments.datasourceName is not configured — skipping") } else { usageReconcilerConf := commitmentsConfig.UsageReconciler usageReconcilerConf.ApplyDefaults() if err := (&commitments.UsageReconciler{ - Client: multiclusterClient, - Conf: usageReconcilerConf, - UsageDB: commitmentsUsageDB, - Monitor: usageReconcilerMonitor, + Client: multiclusterClient, + Conf: usageReconcilerConf, + VMSource: commitmentsVMSource, + Monitor: usageReconcilerMonitor, }).SetupWithManager(mgr, multiclusterClient); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CommittedResourceUsage") os.Exit(1) @@ -701,7 +701,7 @@ func main() { // Create NovaReader and DBVMSource novaReader := external.NewNovaReader(postgresReader) - vmSource := failover.NewDBVMSource(novaReader) + vmSource := reservations.NewDBVMSource(novaReader) // Create the unified failover controller // It handles both: @@ -794,7 +794,7 @@ func main() { // Create NovaReader and DBVMSource novaReader := external.NewNovaReader(postgresReader) - vmSource := failover.NewDBVMSource(novaReader) + vmSource := reservations.NewDBVMSource(novaReader) // Create the quota controller quotaController := quota.NewQuotaController( diff --git a/internal/scheduling/external/nova.go b/internal/scheduling/external/nova.go index 741c5659c..3d12101c0 100644 --- a/internal/scheduling/external/nova.go +++ b/internal/scheduling/external/nova.go @@ -17,6 +17,8 @@ type NovaReaderInterface interface { GetAllFlavors(ctx context.Context) ([]nova.Flavor, error) GetServerByID(ctx context.Context, serverID string) (*nova.Server, error) GetFlavorByName(ctx context.Context, flavorName string) (*nova.Flavor, error) + // GetServersByProject returns all servers belonging to a specific project. + GetServersByProject(ctx context.Context, projectID string) ([]nova.Server, error) // GetDeletedServerByID returns a deleted server by its ID from the deleted_servers table. // Returns nil, nil if the server is not found in the deleted_servers table. GetDeletedServerByID(ctx context.Context, serverID string) (*nova.DeletedServer, error) @@ -83,6 +85,16 @@ func (r *NovaReader) GetAllAggregates(ctx context.Context) ([]nova.Aggregate, er return aggregates, nil } +// GetServersByProject returns all Nova servers belonging to a specific project. +func (r *NovaReader) GetServersByProject(ctx context.Context, projectID string) ([]nova.Server, error) { + var servers []nova.Server + query := "SELECT * FROM " + nova.Server{}.TableName() + " WHERE tenant_id = $1" + if err := r.Select(ctx, &servers, query, projectID); err != nil { + return nil, fmt.Errorf("failed to query servers by project: %w", err) + } + return servers, nil +} + // GetServerByID returns a Nova server by its ID. // Returns nil, nil if the server is not found. func (r *NovaReader) GetServerByID(ctx context.Context, serverID string) (*nova.Server, error) { diff --git a/internal/scheduling/reservations/commitments/api/handler.go b/internal/scheduling/reservations/commitments/api/handler.go index 97a167e90..4c3368ebf 100644 --- a/internal/scheduling/reservations/commitments/api/handler.go +++ b/internal/scheduling/reservations/commitments/api/handler.go @@ -8,6 +8,7 @@ import ( "strings" "sync" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" commitments "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/commitments" "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" @@ -21,7 +22,7 @@ var apiLog = ctrl.Log.WithName("committed-resource-api").WithValues("module", "c type HTTPAPI struct { client client.Client config commitments.APIConfig - usageDB commitments.UsageDBClient + usageDB reservations.VMSource monitor ChangeCommitmentsAPIMonitor usageMonitor ReportUsageAPIMonitor capacityMonitor ReportCapacityAPIMonitor @@ -36,11 +37,11 @@ func NewAPI(client client.Client) *HTTPAPI { } // NewAPIWithConfig creates an HTTPAPI with the given config and optional usageDB client. -func NewAPIWithConfig(k8sClient client.Client, config commitments.APIConfig, usageDB commitments.UsageDBClient) *HTTPAPI { +func NewAPIWithConfig(k8sClient client.Client, config commitments.APIConfig, vmSource reservations.VMSource) *HTTPAPI { return &HTTPAPI{ client: k8sClient, config: config, - usageDB: usageDB, + usageDB: vmSource, monitor: NewChangeCommitmentsAPIMonitor(), usageMonitor: NewReportUsageAPIMonitor(), capacityMonitor: NewReportCapacityAPIMonitor(), diff --git a/internal/scheduling/reservations/commitments/api/report_usage_test.go b/internal/scheduling/reservations/commitments/api/report_usage_test.go index 430dc073d..e25dea790 100644 --- a/internal/scheduling/reservations/commitments/api/report_usage_test.go +++ b/internal/scheduling/reservations/commitments/api/report_usage_test.go @@ -18,6 +18,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" commitments "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/commitments" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "github.com/prometheus/client_golang/prometheus" @@ -572,54 +573,64 @@ type ExpectedVMUsage struct { } // ============================================================================ -// Mock UsageDBClient +// Mock VMSource // ============================================================================ -type mockUsageDBClient struct { - rows map[string][]commitments.VMRow // projectID -> rows - err error +type mockVMSource struct { + vms map[string][]reservations.VM // projectID -> VMs + err error } -func newMockUsageDBClient() *mockUsageDBClient { - return &mockUsageDBClient{ - rows: make(map[string][]commitments.VMRow), - } +func newMockVMSource() *mockVMSource { + return &mockVMSource{vms: make(map[string][]reservations.VM)} } -func (m *mockUsageDBClient) ListProjectVMs(_ context.Context, projectID string) ([]commitments.VMRow, error) { +func (m *mockVMSource) ListVMsByProject(_ context.Context, projectID string) ([]reservations.VM, error) { if m.err != nil { return nil, m.err } - return m.rows[projectID], nil + return m.vms[projectID], nil } -func (m *mockUsageDBClient) addVM(vm *TestVMUsage) { +func (m *mockVMSource) ListVMs(_ context.Context) ([]reservations.VM, error) { return nil, nil } +func (m *mockVMSource) ListVMsOnHypervisors(_ context.Context, _ *hv1.HypervisorList, _ bool) ([]reservations.VM, error) { + return nil, nil +} +func (m *mockVMSource) GetVM(_ context.Context, _ string) (*reservations.VM, error) { return nil, nil } +func (m *mockVMSource) IsServerActive(_ context.Context, _ string) (bool, error) { return false, nil } +func (m *mockVMSource) GetDeletedVMInfo(_ context.Context, _ string) (*reservations.DeletedVMInfo, error) { + return nil, nil +} + +func (m *mockVMSource) addVM(vm *TestVMUsage) { extraSpecs := map[string]string{ "quota:hw_version": vm.Flavor.Group, } if vm.Flavor.VideoRAMMiB != nil { extraSpecs["hw_video:ram_max_mb"] = strconv.FormatUint(*vm.Flavor.VideoRAMMiB, 10) } - extrasJSON, _ := json.Marshal(extraSpecs) //nolint:errcheck // test helper, always valid osType := vm.OSType if osType == "" { osType = "unknown" } - row := commitments.VMRow{ - ID: vm.UUID, - Name: vm.UUID, - Status: "ACTIVE", - Created: vm.CreatedAt.Format(time.RFC3339), - AZ: vm.AZ, - Hypervisor: vm.Host, - FlavorName: vm.Flavor.Name, - FlavorRAM: uint64(vm.Flavor.MemoryMB), //nolint:gosec - FlavorVCPUs: uint64(vm.Flavor.VCPUs), //nolint:gosec - FlavorDisk: vm.Flavor.DiskGB, - FlavorExtras: string(extrasJSON), - OSType: osType, + v := reservations.VM{ + UUID: vm.UUID, + Name: vm.UUID, + Status: "ACTIVE", + CreatedAt: vm.CreatedAt.Format(time.RFC3339), + AvailabilityZone: vm.AZ, + CurrentHypervisor: vm.Host, + FlavorName: vm.Flavor.Name, + FlavorExtraSpecs: extraSpecs, + DiskGB: vm.Flavor.DiskGB, + OSType: osType, + ProjectID: vm.ProjectID, + Resources: map[string]resource.Quantity{ + "memory": *resource.NewQuantity(vm.Flavor.MemoryMB*1024*1024, resource.BinarySI), + "vcpus": *resource.NewQuantity(vm.Flavor.VCPUs, resource.DecimalSI), + }, } - m.rows[vm.ProjectID] = append(m.rows[vm.ProjectID], row) + m.vms[vm.ProjectID] = append(m.vms[vm.ProjectID], v) } // ============================================================================ @@ -630,7 +641,7 @@ type UsageTestEnv struct { T *testing.T Scheme *runtime.Scheme K8sClient client.Client - DBClient *mockUsageDBClient + DBClient *mockVMSource FlavorGroups FlavorGroupsKnowledge HTTPServer *httptest.Server API *HTTPAPI @@ -725,7 +736,7 @@ func newUsageTestEnv( Build() // Create mock DB client with VMs - dbClient := newMockUsageDBClient() + dbClient := newMockVMSource() for _, vm := range vms { dbClient.addVM(vm) } @@ -734,10 +745,10 @@ func newUsageTestEnv( // CalculateUsage reads from this status, so the API returns the correct commitment assignments. if len(crObjects) > 0 { rec := &commitments.UsageReconciler{ - Client: k8sClient, - Conf: commitments.UsageReconcilerConfig{CooldownInterval: metav1.Duration{Duration: 0}}, - UsageDB: dbClient, - Monitor: commitments.NewUsageReconcilerMonitor(), + Client: k8sClient, + Conf: commitments.UsageReconcilerConfig{CooldownInterval: metav1.Duration{Duration: 0}}, + VMSource: dbClient, + Monitor: commitments.NewUsageReconcilerMonitor(), } ctx := context.Background() for _, obj := range crObjects { diff --git a/internal/scheduling/reservations/commitments/api/usage_test.go b/internal/scheduling/reservations/commitments/api/usage_test.go index aca34a49d..eca1f0f9a 100644 --- a/internal/scheduling/reservations/commitments/api/usage_test.go +++ b/internal/scheduling/reservations/commitments/api/usage_test.go @@ -13,6 +13,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" commitments "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/commitments" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "github.com/sapcc/go-api-declarations/liquid" @@ -41,27 +42,42 @@ var testUsageConfig = commitments.APIConfig{ }, } +func mkVM(id, az, flavorName string, ramMiB, vcpus int64, createdAt time.Time, projectID string) reservations.VM { + return reservations.VM{ + UUID: id, + Name: id, + Status: "ACTIVE", + CreatedAt: createdAt.Format(time.RFC3339), + AvailabilityZone: az, + FlavorName: flavorName, + ProjectID: projectID, + Resources: map[string]resource.Quantity{ + "memory": *resource.NewQuantity(ramMiB*1024*1024, resource.BinarySI), + "vcpus": *resource.NewQuantity(vcpus, resource.DecimalSI), + }, + } +} + func TestUsageCalculator_CalculateUsage(t *testing.T) { log.SetLogger(zap.New(zap.WriteTo(os.Stderr), zap.UseDevMode(true))) ctx := context.Background() baseTime := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC) - // Reuse TestFlavor from api_change_commitments_test.go m1Small := &TestFlavor{Name: "m1.small", Group: "hana_1", MemoryMB: 1024, VCPUs: 4} m1Large := &TestFlavor{Name: "m1.large", Group: "hana_1", MemoryMB: 4096, VCPUs: 16} tests := []struct { name string projectID string - vms []commitments.VMRow + vms []reservations.VM reservations []*v1alpha1.Reservation allAZs []liquid.AvailabilityZone - expectedUsage map[string]uint64 // resourceName -> usage + expectedUsage map[string]uint64 }{ { name: "empty project", projectID: "project-empty", - vms: []commitments.VMRow{}, + vms: []reservations.VM{}, reservations: []*v1alpha1.Reservation{}, allAZs: []liquid.AvailabilityZone{"az-a"}, expectedUsage: map[string]uint64{ @@ -71,14 +87,7 @@ func TestUsageCalculator_CalculateUsage(t *testing.T) { { name: "single VM with commitment", projectID: "project-A", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "m1.large", FlavorRAM: 4096, FlavorVCPUs: 16, - }, - }, + vms: []reservations.VM{mkVM("vm-001", "az-a", "m1.large", 4096, 16, baseTime, "project-A")}, reservations: []*v1alpha1.Reservation{ makeUsageTestReservation("commit-1", "project-A", "hana_1", "az-a", 1024*1024*1024, 0), makeUsageTestReservation("commit-1", "project-A", "hana_1", "az-a", 1024*1024*1024, 1), @@ -91,17 +100,10 @@ func TestUsageCalculator_CalculateUsage(t *testing.T) { }, }, { - name: "VM without matching commitment - PAYG", - projectID: "project-B", - vms: []commitments.VMRow{ - { - ID: "vm-002", Name: "vm-002", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "m1.large", FlavorRAM: 4096, FlavorVCPUs: 16, - }, - }, - reservations: []*v1alpha1.Reservation{}, // No commitments + name: "VM without matching commitment - PAYG", + projectID: "project-B", + vms: []reservations.VM{mkVM("vm-002", "az-a", "m1.large", 4096, 16, baseTime, "project-B")}, + reservations: []*v1alpha1.Reservation{}, allAZs: []liquid.AvailabilityZone{"az-a"}, expectedUsage: map[string]uint64{ "hw_version_hana_1_ram": 4, @@ -111,7 +113,6 @@ func TestUsageCalculator_CalculateUsage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Setup K8s client scheme := runtime.NewScheme() _ = v1alpha1.AddToScheme(scheme) _ = hv1.AddToScheme(scheme) @@ -121,7 +122,6 @@ func TestUsageCalculator_CalculateUsage(t *testing.T) { objects = append(objects, r) } - // Build flavor groups using existing test helpers flavorGroups := TestFlavorGroup{ infoVersion: 1234, flavors: []compute.FlavorInGroup{m1Small.ToFlavorInGroup(), m1Large.ToFlavorInGroup()}, @@ -140,35 +140,25 @@ func TestUsageCalculator_CalculateUsage(t *testing.T) { }). Build() - // Setup mock Nova client - dbClient := &mockUsageDBClient{ - rows: map[string][]commitments.VMRow{ - tt.projectID: tt.vms, - }, - } + vmSrc := &mockVMSource{vms: map[string][]reservations.VM{tt.projectID: tt.vms}} - // Create calculator and run - calc := commitments.NewUsageCalculator(k8sClient, dbClient, testUsageConfig) + calc := commitments.NewUsageCalculator(k8sClient, vmSrc, testUsageConfig) logger := log.FromContext(ctx) report, err := calc.CalculateUsage(ctx, logger, tt.projectID, tt.allAZs) if err != nil { t.Fatalf("CalculateUsage failed: %v", err) } - // Verify resource count if len(report.Resources) == 0 { t.Error("Expected at least one resource in report") } - // Verify usage per resource for resourceName, expectedUsage := range tt.expectedUsage { res, ok := report.Resources[liquid.ResourceName(resourceName)] if !ok { t.Errorf("Resource %s not found", resourceName) continue } - - // Sum usage across all AZs var totalUsage uint64 for _, azReport := range res.PerAZ { totalUsage += azReport.Usage @@ -193,22 +183,15 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { tests := []struct { name string projectID string - vms []commitments.VMRow + vms []reservations.VM reservations []*v1alpha1.Reservation allAZs []liquid.AvailabilityZone - expectedActiveCommitment string // non-empty if VM should be assigned to a commitment + expectedActiveCommitment string }{ { name: "active commitment - within time range", projectID: "project-A", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "m1.large", FlavorRAM: 4096, FlavorVCPUs: 16, - }, - }, + vms: []reservations.VM{mkVM("vm-001", "az-a", "m1.large", 4096, 16, baseTime, "project-A")}, reservations: func() []*v1alpha1.Reservation { past := now.Add(-1 * time.Hour) future := now.Add(1 * time.Hour) @@ -225,67 +208,42 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { { name: "expired commitment - should be ignored (VM goes to PAYG)", projectID: "project-A", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "m1.large", FlavorRAM: 4096, FlavorVCPUs: 16, - }, - }, + vms: []reservations.VM{mkVM("vm-001", "az-a", "m1.large", 4096, 16, baseTime, "project-A")}, reservations: func() []*v1alpha1.Reservation { past := now.Add(-2 * time.Hour) - expired := now.Add(-1 * time.Hour) // Already expired + expired := now.Add(-1 * time.Hour) return []*v1alpha1.Reservation{ makeUsageTestReservationWithTimes("commit-expired", "project-A", "hana_1", "az-a", 4*1024*1024*1024, 0, &past, &expired), } }(), allAZs: []liquid.AvailabilityZone{"az-a"}, - expectedActiveCommitment: "", // PAYG - expired commitment ignored + expectedActiveCommitment: "", }, { name: "future commitment - should be ignored (VM goes to PAYG)", projectID: "project-A", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "m1.large", FlavorRAM: 4096, FlavorVCPUs: 16, - }, - }, + vms: []reservations.VM{mkVM("vm-001", "az-a", "m1.large", 4096, 16, baseTime, "project-A")}, reservations: func() []*v1alpha1.Reservation { - futureStart := now.Add(1 * time.Hour) // Hasn't started yet + futureStart := now.Add(1 * time.Hour) futureEnd := now.Add(24 * time.Hour) return []*v1alpha1.Reservation{ makeUsageTestReservationWithTimes("commit-future", "project-A", "hana_1", "az-a", 4*1024*1024*1024, 0, &futureStart, &futureEnd), } }(), allAZs: []liquid.AvailabilityZone{"az-a"}, - expectedActiveCommitment: "", // PAYG - future commitment ignored + expectedActiveCommitment: "", }, { name: "mixed - only active commitment is used", projectID: "project-A", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "m1.large", FlavorRAM: 4096, FlavorVCPUs: 16, - }, - }, + vms: []reservations.VM{mkVM("vm-001", "az-a", "m1.large", 4096, 16, baseTime, "project-A")}, reservations: func() []*v1alpha1.Reservation { - // Expired commitment expiredStart := now.Add(-48 * time.Hour) expiredEnd := now.Add(-24 * time.Hour) - // Active commitment activeStart := now.Add(-1 * time.Hour) activeEnd := now.Add(24 * time.Hour) - // Future commitment futureStart := now.Add(24 * time.Hour) futureEnd := now.Add(48 * time.Hour) - return []*v1alpha1.Reservation{ makeUsageTestReservationWithTimes("commit-expired", "project-A", "hana_1", "az-a", 4*1024*1024*1024, 0, &expiredStart, &expiredEnd), makeUsageTestReservationWithTimes("commit-active", "project-A", "hana_1", "az-a", 4*1024*1024*1024, 0, &activeStart, &activeEnd), @@ -296,7 +254,7 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { } }(), allAZs: []liquid.AvailabilityZone{"az-a"}, - expectedActiveCommitment: "commit-active", // Only active commitment is used + expectedActiveCommitment: "commit-active", }, } @@ -337,14 +295,8 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { }). Build() - dbClient := &mockUsageDBClient{ - rows: map[string][]commitments.VMRow{ - tt.projectID: tt.vms, - }, - } + vmSrc := &mockVMSource{vms: map[string][]reservations.VM{tt.projectID: tt.vms}} - // Create CommittedResource CRDs and run the usage reconciler so that - // CalculateUsage can read pre-computed assignments from CRD status. seen := make(map[string]bool) for _, r := range tt.reservations { if r.Spec.CommittedResourceReservation == nil { @@ -391,10 +343,10 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { t.Fatalf("failed to update CommittedResource status %s: %v", uuid, err) } rec := &commitments.UsageReconciler{ - Client: k8sClient, - Conf: commitments.UsageReconcilerConfig{CooldownInterval: metav1.Duration{Duration: 0}}, - UsageDB: dbClient, - Monitor: commitments.NewUsageReconcilerMonitor(), + Client: k8sClient, + Conf: commitments.UsageReconcilerConfig{CooldownInterval: metav1.Duration{Duration: 0}}, + VMSource: vmSrc, + Monitor: commitments.NewUsageReconcilerMonitor(), } req := ctrl.Request{NamespacedName: types.NamespacedName{Name: cr.Name}} if _, err := rec.Reconcile(ctx, req); err != nil { @@ -402,15 +354,13 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { } } - calc := commitments.NewUsageCalculator(k8sClient, dbClient, testUsageConfig) + calc := commitments.NewUsageCalculator(k8sClient, vmSrc, testUsageConfig) logger := log.FromContext(ctx) report, err := calc.CalculateUsage(ctx, logger, tt.projectID, tt.allAZs) if err != nil { t.Fatalf("CalculateUsage failed: %v", err) } - // Find the VM in subresources and check its commitment assignment - // Subresources are now on the instances resource, not RAM res, ok := report.Resources["hw_version_hana_1_instances"] if !ok { t.Fatal("Resource hw_version_hana_1_instances not found") @@ -422,7 +372,6 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { if sub.Attributes == nil { continue } - // Parse JSON attributes var attrMap map[string]any if err := json.Unmarshal(sub.Attributes, &attrMap); err != nil { continue @@ -432,12 +381,10 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { } if tt.expectedActiveCommitment == "" { - // Expect PAYG (nil commitment_id) if foundCommitment != nil { t.Errorf("Expected PAYG (nil commitment_id), got %v", foundCommitment) } } else { - // Expect specific commitment if foundCommitment != tt.expectedActiveCommitment { t.Errorf("Expected commitment %s, got %v", tt.expectedActiveCommitment, foundCommitment) } @@ -446,9 +393,6 @@ func TestUsageCalculator_ExpiredAndFutureCommitments(t *testing.T) { } } -// TestUsageMultipleCalculation_FloorDivision tests that RAM usage is calculated -// correctly via integer division for variable-ratio flavor groups. -// Nova flavor memory is a multiple of 1024 MiB, so FlavorRAM/1024 gives exact GiB. func TestUsageMultipleCalculation_FloorDivision(t *testing.T) { log.SetLogger(zap.New(zap.WriteTo(os.Stderr), zap.UseDevMode(true))) ctx := context.Background() @@ -461,69 +405,33 @@ func TestUsageMultipleCalculation_FloorDivision(t *testing.T) { tests := []struct { name string - vms []commitments.VMRow - expectedRAM uint64 // Expected RAM usage in units - expectedCores uint64 // Expected cores usage + vms []reservations.VM + expectedRAM uint64 + expectedCores uint64 expectedInstances uint64 }{ { - name: "single smallest flavor - 2 units", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "g_k_c1_m2_v2", FlavorRAM: 2048, FlavorVCPUs: 1, - }, - }, + name: "single smallest flavor - 2 units", + vms: []reservations.VM{mkVM("vm-001", "az-a", "g_k_c1_m2_v2", 2048, 1, baseTime, "project-A")}, expectedRAM: 2, expectedCores: 1, expectedInstances: 1, }, { - name: "2x flavor - 4096/1024 = 4 GiB", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "g_k_c2_m4_v2", FlavorRAM: 4096, FlavorVCPUs: 2, - }, - }, - expectedRAM: 4, // 4096/1024 = 4 + name: "2x flavor - 4096/1024 = 4 GiB", + vms: []reservations.VM{mkVM("vm-001", "az-a", "g_k_c2_m4_v2", 4096, 2, baseTime, "project-A")}, + expectedRAM: 4, expectedCores: 2, expectedInstances: 1, }, { name: "multiple VMs - RAM units should match cores for fixed ratio", - vms: []commitments.VMRow{ - { - ID: "vm-001", Name: "vm-001", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Format(time.RFC3339), - FlavorName: "g_k_c1_m2_v2", FlavorRAM: 2048, FlavorVCPUs: 1, - }, - { - ID: "vm-002", Name: "vm-002", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Add(time.Second).Format(time.RFC3339), - FlavorName: "g_k_c2_m4_v2", FlavorRAM: 4096, FlavorVCPUs: 2, - }, - { - ID: "vm-003", Name: "vm-003", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Add(2 * time.Second).Format(time.RFC3339), - FlavorName: "g_k_c4_m16_v2", FlavorRAM: 16384, FlavorVCPUs: 4, - }, - { - ID: "vm-004", Name: "vm-004", Status: "ACTIVE", - AZ: "az-a", - Created: baseTime.Add(3 * time.Second).Format(time.RFC3339), - FlavorName: "g_k_c16_m32_v2", FlavorRAM: 32768, FlavorVCPUs: 16, - }, + vms: []reservations.VM{ + mkVM("vm-001", "az-a", "g_k_c1_m2_v2", 2048, 1, baseTime, "project-A"), + mkVM("vm-002", "az-a", "g_k_c2_m4_v2", 4096, 2, baseTime.Add(time.Second), "project-A"), + mkVM("vm-003", "az-a", "g_k_c4_m16_v2", 16384, 4, baseTime.Add(2*time.Second), "project-A"), + mkVM("vm-004", "az-a", "g_k_c16_m32_v2", 32768, 16, baseTime.Add(3*time.Second), "project-A"), }, - // 2048/1024 + 4096/1024 + 16384/1024 + 32768/1024 = 2 + 4 + 16 + 32 = 54 - // Cores: 1 + 2 + 4 + 16 = 23 expectedRAM: 54, // 2 + 4 + 16 + 32 expectedCores: 23, // 1 + 2 + 4 + 16 expectedInstances: 4, @@ -536,7 +444,6 @@ func TestUsageMultipleCalculation_FloorDivision(t *testing.T) { _ = v1alpha1.AddToScheme(scheme) _ = hv1.AddToScheme(scheme) - // Build flavor groups with realistic values flavorGroups := TestFlavorGroup{ infoVersion: 1234, flavors: []compute.FlavorInGroup{ @@ -560,20 +467,15 @@ func TestUsageMultipleCalculation_FloorDivision(t *testing.T) { }). Build() - dbClient := &mockUsageDBClient{ - rows: map[string][]commitments.VMRow{ - "project-A": tt.vms, - }, - } + vmSrc := &mockVMSource{vms: map[string][]reservations.VM{"project-A": tt.vms}} - calc := commitments.NewUsageCalculator(k8sClient, dbClient, testUsageConfig) + calc := commitments.NewUsageCalculator(k8sClient, vmSrc, testUsageConfig) logger := log.FromContext(ctx) report, err := calc.CalculateUsage(ctx, logger, "project-A", []liquid.AvailabilityZone{"az-a"}) if err != nil { t.Fatalf("CalculateUsage failed: %v", err) } - // Check RAM usage ramResource := report.Resources[liquid.ResourceName("hw_version_hw_2101_ram")] if ramResource == nil { t.Fatal("hw_version_hw_2101_ram resource not found") @@ -586,7 +488,6 @@ func TestUsageMultipleCalculation_FloorDivision(t *testing.T) { t.Errorf("RAM usage = %d, expected %d", totalRAM, tt.expectedRAM) } - // Check cores usage coresResource := report.Resources[liquid.ResourceName("hw_version_hw_2101_cores")] if coresResource == nil { t.Fatal("hw_version_hw_2101_cores resource not found") @@ -599,7 +500,6 @@ func TestUsageMultipleCalculation_FloorDivision(t *testing.T) { t.Errorf("Cores usage = %d, expected %d", totalCores, tt.expectedCores) } - // Check instances usage instancesResource := report.Resources[liquid.ResourceName("hw_version_hw_2101_instances")] if instancesResource == nil { t.Fatal("hw_version_hw_2101_instances resource not found") @@ -620,7 +520,7 @@ func makeUsageTestReservation(commitmentUUID, projectID, flavorGroup, az string, } func makeUsageTestReservationWithTimes(commitmentUUID, projectID, flavorGroup, az string, memoryBytes int64, slot int, startTime, endTime *time.Time) *v1alpha1.Reservation { - name := "commitment-" + commitmentUUID + "-" + string(rune('0'+slot)) //nolint:gosec // slot is a small test index, no overflow risk + name := "commitment-" + commitmentUUID + "-" + string(rune('0'+slot)) //nolint:gosec res := &v1alpha1.Reservation{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/scheduling/reservations/commitments/usage.go b/internal/scheduling/reservations/commitments/usage.go index fb9984a75..ccc0aaca8 100644 --- a/internal/scheduling/reservations/commitments/usage.go +++ b/internal/scheduling/reservations/commitments/usage.go @@ -5,18 +5,14 @@ package commitments import ( "context" - "encoding/json" "errors" "fmt" "sort" "strconv" - "sync" "time" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/knowledge/datasources/plugins/openstack/nova" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - "github.com/cobaltcore-dev/cortex/internal/scheduling/external" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" "github.com/go-logr/logr" "github.com/sapcc/go-api-declarations/liquid" @@ -24,38 +20,28 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// UsageDBClient is the minimal interface for querying VM usage data from Postgres. -type UsageDBClient interface { - // ListProjectVMs returns all VMs for a project with their flavor data. - ListProjectVMs(ctx context.Context, projectID string) ([]VMRow, error) +// UsageCalculator computes usage reports for Limes LIQUID API. +type UsageCalculator struct { + client client.Client + vmSource reservations.VMSource + config APIConfig } -// VMRow is the result of a joined server+flavor+image query from Postgres. -type VMRow struct { - ID string - Name string - Status string - Created string - AZ string - Hypervisor string - FlavorName string - FlavorRAM uint64 - FlavorVCPUs uint64 - FlavorDisk uint64 - FlavorExtras string // JSON string of flavor extra_specs - OSType string // pre-computed from Glance image properties; "unknown" when not found +// NewUsageCalculator creates a new UsageCalculator instance. +func NewUsageCalculator(client client.Client, vmSource reservations.VMSource, config APIConfig) *UsageCalculator { + return &UsageCalculator{ + client: client, + vmSource: vmSource, + config: config, + } } // CommitmentStateWithUsage extends CommitmentState with usage tracking for billing calculations. -// Used by the report-usage API to track remaining capacity during VM-to-commitment assignment. type CommitmentStateWithUsage struct { CommitmentState - // RemainingMemoryBytes is the uncommitted capacity left for VM assignment RemainingMemoryBytes int64 - // AssignedInstances tracks which VM instances have been assigned to this commitment - AssignedInstances []string - // UsedVCPUs is the total vCPU count of assigned VM instances - UsedVCPUs int64 + AssignedInstances []string + UsedVCPUs int64 } // NewCommitmentStateWithUsage creates a CommitmentStateWithUsage from a CommitmentState. @@ -68,7 +54,6 @@ func NewCommitmentStateWithUsage(state *CommitmentState) *CommitmentStateWithUsa } // AssignVM attempts to assign a VM to this commitment if there's enough capacity. -// Returns true if the VM was assigned, false if not enough capacity. func (c *CommitmentStateWithUsage) AssignVM(vmUUID string, vmMemoryBytes, vCPUs int64) bool { if c.RemainingMemoryBytes >= vmMemoryBytes { c.RemainingMemoryBytes -= vmMemoryBytes @@ -84,8 +69,7 @@ func (c *CommitmentStateWithUsage) HasRemainingCapacity() bool { return c.RemainingMemoryBytes > 0 } -// VMUsageInfo contains VM information needed for usage calculation. -// This is a local view of the VM enriched with flavor group information. +// VMUsageInfo contains VM information needed for usage calculation, enriched with flavor group data. type VMUsageInfo struct { UUID string Name string @@ -95,28 +79,12 @@ type VMUsageInfo struct { MemoryMB uint64 VCPUs uint64 DiskGB uint64 - VideoRAMMiB *uint64 // optional, from flavor extra_specs hw_video:ram_max_mb - OSType string // pre-computed from Glance image; "unknown" for volume-booted or unmapped images + VideoRAMMiB *uint64 + OSType string AZ string Hypervisor string CreatedAt time.Time - UsageMultiple uint64 // RAM in the group's declared unit: slot count (fixed-ratio) or GiB (variable-ratio) -} - -// UsageCalculator computes usage reports for Limes LIQUID API. -type UsageCalculator struct { - client client.Client - usageDB UsageDBClient - config APIConfig -} - -// NewUsageCalculator creates a new UsageCalculator instance. -func NewUsageCalculator(client client.Client, usageDB UsageDBClient, config APIConfig) *UsageCalculator { - return &UsageCalculator{ - client: client, - usageDB: usageDB, - config: config, - } + UsageMultiple uint64 } // CalculateUsage computes the usage report for a specific project. @@ -155,7 +123,7 @@ func (c *UsageCalculator) CalculateUsage( quotaByResourceAZ = buildCombinedQuotaMap(pqList.Items) } - vms, err := getProjectVMs(ctx, c.usageDB, log, projectID, flavorGroups, allAZs) + vms, err := getProjectVMs(ctx, c.vmSource, log, projectID, flavorGroups, allAZs) if err != nil { return liquid.ServiceUsageReport{}, fmt.Errorf("failed to get project VMs: %w", err) } @@ -266,23 +234,23 @@ func buildCommitmentCapacityMap( return result, nil } -// getProjectVMs retrieves all VMs for a project from Postgres and enriches them with flavor group info. +// getProjectVMs retrieves all VMs for a project via VMSource and enriches them with flavor group info. func getProjectVMs( ctx context.Context, - usageDB UsageDBClient, + vmSource reservations.VMSource, log logr.Logger, projectID string, flavorGroups map[string]compute.FlavorGroupFeature, allAZs []liquid.AvailabilityZone, ) ([]VMUsageInfo, error) { - if usageDB == nil { - return nil, errors.New("usage DB client not configured") + if vmSource == nil { + return nil, errors.New("vm source not configured") } - rows, err := usageDB.ListProjectVMs(ctx, projectID) + projectVMs, err := vmSource.ListVMsByProject(ctx, projectID) if err != nil { - return nil, fmt.Errorf("failed to list VMs from Postgres: %w", err) + return nil, fmt.Errorf("failed to list VMs from source: %w", err) } // Build flavor name -> flavor group lookup @@ -294,59 +262,56 @@ func getProjectVMs( } var vms []VMUsageInfo - for _, row := range rows { - // Parse creation time (Nova returns ISO 8601/RFC3339 format) - createdAt, err := time.Parse(time.RFC3339, row.Created) + for _, vm := range projectVMs { + createdAt, err := time.Parse(time.RFC3339, vm.CreatedAt) if err != nil { log.V(1).Info("failed to parse server creation time, using zero time", - "server", row.ID, "created", row.Created, "error", err.Error()) + "server", vm.UUID, "created", vm.CreatedAt, "error", err.Error()) createdAt = time.Time{} } - // Determine flavor group - flavorGroup := flavorToGroup[row.FlavorName] + flavorGroup := flavorToGroup[vm.FlavorName] + + memoryMB := uint64(0) + vcpus := uint64(0) + if qty, ok := vm.Resources["memory"]; ok { + memoryMB = uint64(qty.Value()) / (1024 * 1024) //nolint:gosec + } + if qty, ok := vm.Resources["vcpus"]; ok { + vcpus = uint64(qty.Value()) //nolint:gosec + } - // Compute usage in the unit declared by the info endpoint for this group: - // - fixed-ratio: slot count (FlavorRAM / smallest.MemoryMB); exact since flavors are integer multiples - // - variable-ratio or unknown: GiB, +16 MiB before dividing to handle flavors that reserve 16 MiB for video RAM var usageMultiple uint64 - if row.FlavorRAM > 0 { + if memoryMB > 0 { if fg, ok := flavorGroups[flavorGroup]; ok && fg.HasFixedRamCoreRatio() { - usageMultiple = row.FlavorRAM / fg.SmallestFlavor.MemoryMB + usageMultiple = memoryMB / fg.SmallestFlavor.MemoryMB } else { - usageMultiple = (row.FlavorRAM + 16) / 1024 + usageMultiple = (memoryMB + 16) / 1024 } } - // Normalize AZ - normalizedAZ := liquid.NormalizeAZ(row.AZ, allAZs) + normalizedAZ := liquid.NormalizeAZ(vm.AvailabilityZone, allAZs) - // Parse video RAM from flavor extra_specs var videoRAMMiB *uint64 - if row.FlavorExtras != "" { - var extraSpecs map[string]string - if err := json.Unmarshal([]byte(row.FlavorExtras), &extraSpecs); err == nil { - if val, ok := extraSpecs["hw_video:ram_max_mb"]; ok { - if parsed, err := strconv.ParseUint(val, 10, 64); err == nil { - videoRAMMiB = &parsed - } - } + if val, ok := vm.FlavorExtraSpecs["hw_video:ram_max_mb"]; ok { + if parsed, err := strconv.ParseUint(val, 10, 64); err == nil { + videoRAMMiB = &parsed } } vms = append(vms, VMUsageInfo{ - UUID: row.ID, - Name: row.Name, - FlavorName: row.FlavorName, + UUID: vm.UUID, + Name: vm.Name, + FlavorName: vm.FlavorName, FlavorGroup: flavorGroup, - Status: row.Status, - MemoryMB: row.FlavorRAM, - VCPUs: row.FlavorVCPUs, - DiskGB: row.FlavorDisk, + Status: vm.Status, + MemoryMB: memoryMB, + VCPUs: vcpus, + DiskGB: vm.DiskGB, VideoRAMMiB: videoRAMMiB, - OSType: row.OSType, + OSType: vm.OSType, AZ: string(normalizedAZ), - Hypervisor: row.Hypervisor, + Hypervisor: vm.CurrentHypervisor, CreatedAt: createdAt, UsageMultiple: usageMultiple, }) @@ -646,80 +611,3 @@ func countCommitmentStates(m map[string][]*CommitmentStateWithUsage) int { } return count } - -// dbUsageClient implements UsageDBClient using a lazy-connecting PostgresReader. -type dbUsageClient struct { - k8sClient client.Client - datasourceName string - mu sync.Mutex - reader *external.PostgresReader -} - -// NewDBUsageClient creates a UsageDBClient that lazily connects to Postgres via the named Datasource CRD. -func NewDBUsageClient(k8sClient client.Client, datasourceName string) UsageDBClient { - return &dbUsageClient{k8sClient: k8sClient, datasourceName: datasourceName} -} - -func (c *dbUsageClient) getReader(ctx context.Context) (*external.PostgresReader, error) { - c.mu.Lock() - defer c.mu.Unlock() - if c.reader != nil { - return c.reader, nil - } - reader, err := external.NewPostgresReader(ctx, c.k8sClient, c.datasourceName) - if err != nil { - return nil, fmt.Errorf("failed to connect to datasource %s: %w", c.datasourceName, err) - } - c.reader = reader - return reader, nil -} - -// vmQueryRow is the scan target for the server+flavor+image JOIN query. -type vmQueryRow struct { - ID string `db:"id"` - Name string `db:"name"` - Status string `db:"status"` - Created string `db:"created"` - AZ string `db:"az"` - Hypervisor string `db:"hypervisor"` - FlavorName string `db:"flavor_name"` - FlavorRAM uint64 `db:"flavor_ram"` - FlavorVCPUs uint64 `db:"flavor_vcpus"` - FlavorDisk uint64 `db:"flavor_disk"` - FlavorExtras string `db:"flavor_extras"` - OSType string `db:"os_type"` -} - -// ListProjectVMs returns all VMs for a project joined with their flavor data from Postgres. -func (c *dbUsageClient) ListProjectVMs(ctx context.Context, projectID string) ([]VMRow, error) { - reader, err := c.getReader(ctx) - if err != nil { - return nil, err - } - - query := ` - SELECT - s.id, s.name, s.status, s.created, - s.os_ext_az_availability_zone AS az, - s.os_ext_srv_attr_hypervisor_hostname AS hypervisor, - s.flavor_name, - COALESCE(f.ram, 0) AS flavor_ram, - COALESCE(f.vcpus, 0) AS flavor_vcpus, - COALESCE(f.disk, 0) AS flavor_disk, - COALESCE(f.extra_specs, '') AS flavor_extras, - COALESCE(NULLIF(s.os_type, ''), 'unknown') AS os_type - FROM ` + nova.Server{}.TableName() + ` s - LEFT JOIN ` + nova.Flavor{}.TableName() + ` f ON f.name = s.flavor_name - WHERE s.tenant_id = $1` - - var rows []vmQueryRow - if err := reader.Select(ctx, &rows, query, projectID); err != nil { - return nil, fmt.Errorf("failed to query VMs for project %s: %w", projectID, err) - } - - result := make([]VMRow, len(rows)) - for i, r := range rows { - result[i] = VMRow(r) - } - return result, nil -} diff --git a/internal/scheduling/reservations/commitments/usage_reconciler.go b/internal/scheduling/reservations/commitments/usage_reconciler.go index 3b369a848..28d23b7ff 100644 --- a/internal/scheduling/reservations/commitments/usage_reconciler.go +++ b/internal/scheduling/reservations/commitments/usage_reconciler.go @@ -33,9 +33,9 @@ import ( // relevant change events. type UsageReconciler struct { client.Client - Conf UsageReconcilerConfig - UsageDB UsageDBClient - Monitor UsageReconcilerMonitor + Conf UsageReconcilerConfig + VMSource reservations.VMSource + Monitor UsageReconcilerMonitor } func (r *UsageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -155,7 +155,7 @@ func (r *UsageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } } - vms, err := getProjectVMs(ctx, r.UsageDB, logger, cr.Spec.ProjectID, flavorGroups, allAZs) + vms, err := getProjectVMs(ctx, r.VMSource, logger, cr.Spec.ProjectID, flavorGroups, allAZs) if err != nil { r.Monitor.reconcileDuration.WithLabelValues("error").Observe(time.Since(start).Seconds()) return ctrl.Result{}, err diff --git a/internal/scheduling/reservations/failover/controller.go b/internal/scheduling/reservations/failover/controller.go index c66295293..04dd8d401 100644 --- a/internal/scheduling/reservations/failover/controller.go +++ b/internal/scheduling/reservations/failover/controller.go @@ -39,7 +39,7 @@ var log = ctrl.Log.WithName("failover-reservation-controller").WithValues("modul // 2. Watch-based per-reservation reconciliation (Reconcile) - handles acknowledgment and validation of individual reservations type FailoverReservationController struct { client.Client - VMSource VMSource + VMSource reservations.VMSource Config FailoverConfig SchedulerClient *reservations.SchedulerClient Recorder events.EventRecorder // Event recorder for emitting Kubernetes events @@ -47,7 +47,7 @@ type FailoverReservationController struct { reconcileCount int64 // Track reconciliation count for rotating VM selection } -func NewFailoverReservationController(c client.Client, vmSource VMSource, config FailoverConfig, schedulerClient *reservations.SchedulerClient, monitor *FailoverMonitor) *FailoverReservationController { +func NewFailoverReservationController(c client.Client, vmSource reservations.VMSource, config FailoverConfig, schedulerClient *reservations.SchedulerClient, monitor *FailoverMonitor) *FailoverReservationController { return &FailoverReservationController{ Client: c, VMSource: vmSource, @@ -58,7 +58,7 @@ func NewFailoverReservationController(c client.Client, vmSource VMSource, config } type vmFailoverNeed struct { - VM VM + VM reservations.VM Count int // Number of failover reservations needed } @@ -383,7 +383,7 @@ func (c *FailoverReservationController) ReconcilePeriodic(ctx context.Context) ( // The caller is responsible for persisting any changes to the cluster. func reconcileRemoveInvalidVMFromReservations( ctx context.Context, - vms []VM, + vms []reservations.VM, failoverReservations []v1alpha1.Reservation, ) (updatedReservations []v1alpha1.Reservation, reservationsToUpdate []*v1alpha1.Reservation) { @@ -442,13 +442,13 @@ func reconcileRemoveInvalidVMFromReservations( // they no longer meet eligibility criteria. func reconcileRemoveNoneligibleVMFromReservations( ctx context.Context, - vms []VM, + vms []reservations.VM, failoverReservations []v1alpha1.Reservation, ) (updatedReservations []v1alpha1.Reservation, reservationsToUpdate []*v1alpha1.Reservation) { logger := LoggerFromContext(ctx) - vmByUUID := make(map[string]VM) + vmByUUID := make(map[string]reservations.VM) for _, vm := range vms { vmByUUID[vm.UUID] = vm } @@ -584,7 +584,7 @@ func sortVMsByMemory(vms []vmFailoverNeed) { // reconcileCreateAndAssignReservations creates and assigns failover reservations for VMs that need them. func (c *FailoverReservationController) reconcileCreateAndAssignReservations( ctx context.Context, - vms []VM, + vms []reservations.VM, failoverReservations []v1alpha1.Reservation, allHypervisors []string, flavorGroups map[string]compute.FlavorGroupFeature, // passed to resolveVMForScheduling per-VM @@ -703,7 +703,7 @@ func (c *FailoverReservationController) reconcileCreateAndAssignReservations( // calculateVMsMissingFailover calculates which VMs need failover reservations and how many. func (c *FailoverReservationController) calculateVMsMissingFailover( ctx context.Context, - vms []VM, + vms []reservations.VM, failoverReservations []v1alpha1.Reservation, ) []vmFailoverNeed { diff --git a/internal/scheduling/reservations/failover/controller_test.go b/internal/scheduling/reservations/failover/controller_test.go index 78d3168c7..482fa4f30 100644 --- a/internal/scheduling/reservations/failover/controller_test.go +++ b/internal/scheduling/reservations/failover/controller_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,7 +21,7 @@ import ( func TestReconcileRemoveNoneligibleVMFromReservations(t *testing.T) { tests := []struct { name string - vms []VM + vms []reservations.VM reservations []v1alpha1.Reservation expectedUpdatedCount int expectedToUpdateCount int @@ -28,7 +29,7 @@ func TestReconcileRemoveNoneligibleVMFromReservations(t *testing.T) { }{ { name: "no changes needed - all VMs eligible", - vms: []VM{ + vms: []reservations.VM{ newTestVMWithResources("vm-1", "host1"), newTestVMWithResources("vm-2", "host2"), }, @@ -46,7 +47,7 @@ func TestReconcileRemoveNoneligibleVMFromReservations(t *testing.T) { }, { name: "VM on same host as reservation - remove", - vms: []VM{ + vms: []reservations.VM{ newTestVMWithResources("vm-1", "host3"), // VM moved to host3 (same as reservation) }, reservations: []v1alpha1.Reservation{ @@ -62,7 +63,7 @@ func TestReconcileRemoveNoneligibleVMFromReservations(t *testing.T) { }, { name: "multiple ineligible VMs - all processed", - vms: []VM{ + vms: []reservations.VM{ newTestVMWithResources("vm-1", "host3"), // ineligible (on same host as res-1) newTestVMWithResources("vm-2", "host4"), // ineligible (on same host as res-2) }, @@ -83,7 +84,7 @@ func TestReconcileRemoveNoneligibleVMFromReservations(t *testing.T) { }, { name: "VM not in list - keep in allocations", - vms: []VM{ + vms: []reservations.VM{ newTestVMWithResources("vm-1", "host1"), // vm-2 not in list }, @@ -218,167 +219,6 @@ func TestFilterFailoverReservations(t *testing.T) { } } -// ============================================================================ -// Test: filterVMsOnKnownHypervisors -// ============================================================================ - -func TestFilterVMsOnKnownHypervisors(t *testing.T) { - tests := []struct { - name string - vms []VM - hypervisorList *hv1.HypervisorList - expectedCount int - expectedUUIDs []string - }{ - { - name: "empty VMs list", - vms: []VM{}, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{}), - newTestHypervisor("host2", []hv1.Instance{}), - }, - }, - expectedCount: 0, - expectedUUIDs: nil, - }, - { - name: "all VMs on known hypervisors and in instances", - vms: []VM{ - newTestVM("vm-1", "host1", "m1.large"), - newTestVM("vm-2", "host2", "m1.large"), - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{{ID: "vm-1", Name: "vm-1", Active: true}}), - newTestHypervisor("host2", []hv1.Instance{{ID: "vm-2", Name: "vm-2", Active: true}}), - newTestHypervisor("host3", []hv1.Instance{}), - }, - }, - expectedCount: 2, - expectedUUIDs: []string{"vm-1", "vm-2"}, - }, - { - name: "some VMs on unknown hypervisors", - vms: []VM{ - newTestVM("vm-1", "host1", "m1.large"), - newTestVM("vm-2", "unknown-host", "m1.large"), - newTestVM("vm-3", "host2", "m1.large"), - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{{ID: "vm-1", Name: "vm-1", Active: true}}), - newTestHypervisor("host2", []hv1.Instance{{ID: "vm-3", Name: "vm-3", Active: true}}), - }, - }, - expectedCount: 2, - expectedUUIDs: []string{"vm-1", "vm-3"}, - }, - { - name: "VM claims hypervisor but not in instances list - filter out", - vms: []VM{ - newTestVM("vm-1", "host1", "m1.large"), - newTestVM("vm-2", "host2", "m1.large"), // claims host2 but not in instances - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{{ID: "vm-1", Name: "vm-1", Active: true}}), - newTestHypervisor("host2", []hv1.Instance{}), // vm-2 not in instances - }, - }, - expectedCount: 1, - expectedUUIDs: []string{"vm-1"}, - }, - { - name: "VM on wrong hypervisor in instances - filter out", - vms: []VM{ - newTestVM("vm-1", "host1", "m1.large"), // claims host1 - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{}), // vm-1 not here - newTestHypervisor("host2", []hv1.Instance{{ID: "vm-1", Name: "vm-1", Active: true}}), // vm-1 is actually here - }, - }, - expectedCount: 0, - expectedUUIDs: nil, - }, - { - name: "inactive VM in instances - filter out", - vms: []VM{ - newTestVM("vm-1", "host1", "m1.large"), - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{{ID: "vm-1", Name: "vm-1", Active: false}}), // inactive - }, - }, - expectedCount: 0, - expectedUUIDs: nil, - }, - { - name: "no VMs on known hypervisors", - vms: []VM{ - newTestVM("vm-1", "unknown1", "m1.large"), - newTestVM("vm-2", "unknown2", "m1.large"), - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{ - newTestHypervisor("host1", []hv1.Instance{}), - newTestHypervisor("host2", []hv1.Instance{}), - }, - }, - expectedCount: 0, - expectedUUIDs: nil, - }, - { - name: "empty hypervisors list", - vms: []VM{ - newTestVM("vm-1", "host1", "m1.large"), - }, - hypervisorList: &hv1.HypervisorList{ - Items: []hv1.Hypervisor{}, - }, - expectedCount: 0, - expectedUUIDs: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := filterVMsOnKnownHypervisors(tt.vms, tt.hypervisorList) - - if len(result) != tt.expectedCount { - t.Errorf("expected %d VMs, got %d", tt.expectedCount, len(result)) - } - - // Check that expected UUIDs are present (order may vary due to filtering) - resultUUIDs := make(map[string]bool) - for _, vm := range result { - resultUUIDs[vm.UUID] = true - } - - for _, uuid := range tt.expectedUUIDs { - if !resultUUIDs[uuid] { - t.Errorf("expected VM %s in result, but not found", uuid) - } - } - }) - } -} - -// newTestHypervisor creates a test hypervisor with the given instances -func newTestHypervisor(name string, instances []hv1.Instance) hv1.Hypervisor { - return hv1.Hypervisor{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Status: hv1.HypervisorStatus{ - Instances: instances, - }, - } -} - // ============================================================================ // Test: countReservationsForVM // ============================================================================ @@ -576,7 +416,7 @@ func TestGetRequiredFailoverCount(t *testing.T) { func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { tests := []struct { name string - vms []VM + vms []reservations.VM reservations []v1alpha1.Reservation expectedUpdatedCount int // number of reservations in updatedReservations expectedToUpdateCount int // number of reservations that need cluster update @@ -584,9 +424,9 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }{ { name: "no changes needed - all VMs valid", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), - newTestVM("vm-2", "host2", "flavor1"), + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), + newTestVM("vm-2", "host2"), }, reservations: []v1alpha1.Reservation{ newTestReservation("res-1", "host3", map[string]string{ @@ -602,8 +442,8 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "VM no longer exists - remove from allocations", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), // vm-2 no longer exists }, reservations: []v1alpha1.Reservation{ @@ -620,9 +460,9 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "VM moved to different host - remove from allocations", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), - newTestVM("vm-2", "host4", "flavor1"), // moved from host2 to host4 + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), + newTestVM("vm-2", "host4"), // moved from host2 to host4 }, reservations: []v1alpha1.Reservation{ newTestReservation("res-1", "host3", map[string]string{ @@ -638,9 +478,9 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "multiple reservations - only affected ones updated", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), - newTestVM("vm-2", "host2", "flavor1"), + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), + newTestVM("vm-2", "host2"), // vm-3 no longer exists }, reservations: []v1alpha1.Reservation{ @@ -661,7 +501,7 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "all VMs removed from reservation - empty allocations", - vms: []VM{ + vms: []reservations.VM{ // no VMs exist }, reservations: []v1alpha1.Reservation{ @@ -678,8 +518,8 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "empty reservations list", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), }, reservations: []v1alpha1.Reservation{}, expectedUpdatedCount: 0, @@ -688,8 +528,8 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "reservation with no allocations - no changes", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), }, reservations: []v1alpha1.Reservation{ newTestReservation("res-1", "host3", map[string]string{}), @@ -702,11 +542,11 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { }, { name: "mixed scenario - some VMs valid, some deleted, some moved", - vms: []VM{ - newTestVM("vm-1", "host1", "flavor1"), // valid - newTestVM("vm-2", "host5", "flavor1"), // moved from host2 to host5 + vms: []reservations.VM{ + newTestVM("vm-1", "host1"), // valid + newTestVM("vm-2", "host5"), // moved from host2 to host5 // vm-3 deleted - newTestVM("vm-4", "host4", "flavor1"), // valid + newTestVM("vm-4", "host4"), // valid }, reservations: []v1alpha1.Reservation{ newTestReservation("res-1", "host6", map[string]string{ @@ -782,17 +622,17 @@ func TestReconcileRemoveInvalidVMFromReservations(t *testing.T) { // Test helper functions - local to this test file -func newTestVM(uuid, currentHypervisor, flavorName string) VM { - return VM{ +func newTestVM(uuid, currentHypervisor string) reservations.VM { + return reservations.VM{ UUID: uuid, CurrentHypervisor: currentHypervisor, - FlavorName: flavorName, + FlavorName: "flavor1", ProjectID: "test-project", } } -func newTestVMWithResources(uuid, currentHypervisor string) VM { - return VM{ +func newTestVMWithResources(uuid, currentHypervisor string) reservations.VM { + return reservations.VM{ UUID: uuid, CurrentHypervisor: currentHypervisor, FlavorName: "m1.large", @@ -860,7 +700,7 @@ func TestSelectVMsToProcess(t *testing.T) { vms := make([]vmFailoverNeed, count) for i := range count { vms[i] = vmFailoverNeed{ - VM: VM{ + VM: reservations.VM{ UUID: "vm-" + string(rune('a'+i)), CurrentHypervisor: "host" + string(rune('1'+i)), Resources: map[string]resource.Quantity{ diff --git a/internal/scheduling/reservations/failover/helpers.go b/internal/scheduling/reservations/failover/helpers.go index 8623f117f..ef1b47933 100644 --- a/internal/scheduling/reservations/failover/helpers.go +++ b/internal/scheduling/reservations/failover/helpers.go @@ -48,7 +48,7 @@ func (r resolvedReservationSpec) HypervisorResources() map[hv1.ResourceName]reso // returns the LargestFlavor's name and size. Otherwise falls back to VM resources. func resolveVMSpecForScheduling( ctx context.Context, - vm VM, + vm reservations.VM, useFlavorGroupResources bool, flavorGroups map[string]compute.FlavorGroupFeature, ) resolvedReservationSpec { @@ -127,7 +127,7 @@ func countReservationsForVM(resList []v1alpha1.Reservation, vmUUID string) int { // addVMToReservation creates a copy of a reservation with the VM added to its allocations. // The original reservation is NOT modified. -func addVMToReservation(reservation v1alpha1.Reservation, vm VM) *v1alpha1.Reservation { +func addVMToReservation(reservation v1alpha1.Reservation, vm reservations.VM) *v1alpha1.Reservation { // Deep copy the reservation updatedRes := reservation.DeepCopy() @@ -176,7 +176,7 @@ func ValidateFailoverReservationResources(res *v1alpha1.Reservation) error { // This ensures the same sizing is used for both the scheduler query and the reservation CRD. func newFailoverReservation( ctx context.Context, - vm VM, + vm reservations.VM, hypervisor, creator string, resSpec resolvedReservationSpec, ) *v1alpha1.Reservation { diff --git a/internal/scheduling/reservations/failover/integration_test.go b/internal/scheduling/reservations/failover/integration_test.go index df1354be6..6de87a3e0 100644 --- a/internal/scheduling/reservations/failover/integration_test.go +++ b/internal/scheduling/reservations/failover/integration_test.go @@ -41,7 +41,7 @@ type IntegrationTestCase struct { Name string Hypervisors []*hv1.Hypervisor Reservations []*v1alpha1.Reservation - VMs []VM + VMs []reservations.VM FlavorRequirements map[string]int // Verification options @@ -69,7 +69,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host3", 8, 16, 0, 0, nil, nil), newHypervisor("host4", 8, 16, 0, 0, nil, nil), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), }, @@ -89,7 +89,7 @@ func TestIntegration(t *testing.T) { Reservations: []*v1alpha1.Reservation{ newReservation("existing-res-1", "host2", 8192, 4, map[string]string{"vm-1": "host1"}), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), }, @@ -104,7 +104,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host2", 8, 16, 0, 0, nil, nil), newHypervisor("host3", 8, 16, 0, 0, nil, nil), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.small", "project-A", "host1", 4096, 2), }, FlavorRequirements: map[string]int{"m1.large": 1}, // m1.small not in requirements @@ -123,7 +123,7 @@ func TestIntegration(t *testing.T) { Reservations: []*v1alpha1.Reservation{ newReservation("existing-res-1", "host2", 8192, 4, map[string]string{"vm-1": "host1"}), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), newVM("vm-3", "m1.large", "project-A", "host3", 8192, 4), @@ -146,7 +146,7 @@ func TestIntegration(t *testing.T) { newHypervisorWithAZ("host-b1", 16, 32, 4, 8, []hv1.Instance{{ID: "vm-b1", Name: "vm-b1", Active: true}}, nil, "az-b"), newHypervisorWithAZ("host-b2", 16, 32, 0, 0, nil, nil, "az-b"), // Empty host for failover in AZ-B }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithAZ("vm-a1", "m1.large", "project-A", "host-a1", 8192, 4, "az-a"), newVMWithAZ("vm-b1", "m1.large", "project-A", "host-b1", 8192, 4, "az-b"), }, @@ -164,7 +164,7 @@ func TestIntegration(t *testing.T) { newHypervisorWithAZ("host-b1", 16, 32, 0, 0, nil, nil, "az-b"), newHypervisorWithAZ("host-b2", 16, 32, 0, 0, nil, nil, "az-b"), }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithAZ("vm-a1", "m1.large", "project-A", "host-a1", 8192, 4, "az-a"), }, FlavorRequirements: map[string]int{}, // Empty - don't require failover for this test @@ -183,7 +183,7 @@ func TestIntegration(t *testing.T) { newHypervisorWithAZ("host-b1", 16, 32, 4, 8, []hv1.Instance{{ID: "vm-b1", Name: "vm-b1", Active: true}}, nil, "az-b"), newHypervisorWithAZ("host-b2", 16, 32, 0, 0, nil, nil, "az-b"), // Empty host for failover }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithAZ("vm-a1", "m1.large", "project-A", "host-a1", 8192, 4, "az-a"), newVMWithAZ("vm-a2", "m1.large", "project-A", "host-a2", 8192, 4, "az-a"), newVMWithAZ("vm-b1", "m1.large", "project-A", "host-b1", 8192, 4, "az-b"), @@ -209,7 +209,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host7", 32, 64, 0, 0, nil, nil), newHypervisor("host8", 32, 64, 0, 0, nil, nil), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), newVM("vm-3", "m1.large", "project-A", "host3", 8192, 4), @@ -232,7 +232,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host8", 32, 64, 0, 0, nil, nil), newHypervisor("host9", 32, 64, 0, 0, nil, nil), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), newVM("vm-3", "m1.large", "project-A", "host3", 8192, 4), @@ -257,7 +257,7 @@ func TestIntegration(t *testing.T) { newReservation("existing-res-1", "host4", 8192, 4, map[string]string{"vm-1": "host1"}), newReservation("existing-res-2", "host5", 8192, 4, map[string]string{"vm-2": "host2"}), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), newVM("vm-3", "m1.large", "project-A", "host3", 8192, 4), @@ -283,7 +283,7 @@ func TestIntegration(t *testing.T) { "vm-deleted": "host3", // This VM no longer exists }), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), }, FlavorRequirements: map[string]int{"m1.large": 1}, @@ -307,7 +307,7 @@ func TestIntegration(t *testing.T) { "vm-2": "host3", }), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host2", 8192, 4), // Moved from host1 to host2 newVM("vm-2", "m1.large", "project-A", "host3", 8192, 4), }, @@ -332,7 +332,7 @@ func TestIntegration(t *testing.T) { "vm-2": "host2", }), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host3", 8192, 4), // Same as reservation! newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), }, @@ -358,7 +358,7 @@ func TestIntegration(t *testing.T) { "vm-deleted": "host4", // Deleted }), }, - VMs: []VM{ + VMs: []reservations.VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), newVM("vm-2", "m1.large", "project-A", "host2", 8192, 4), // Moved from host3 to host2 }, @@ -381,7 +381,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host3", 16, 32, 4, 8, []hv1.Instance{{ID: "vm-regular-1", Name: "vm-regular-1", Active: true}}, nil), newHypervisor("host4", 16, 32, 0, 0, nil, nil), }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithExtraSpecs("vm-hana-1", "m1.hana", "project-A", "host1", 8192, 4, map[string]string{"trait:CUSTOM_HANA": "required"}), newVMWithExtraSpecs("vm-regular-1", "m1.large", "project-A", "host3", 8192, 4, nil), }, @@ -396,7 +396,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host2", 16, 32, 0, 0, nil, []string{"CUSTOM_HANA"}), // Has HANA trait newHypervisor("host3", 16, 32, 0, 0, nil, nil), // No HANA trait }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithExtraSpecs("vm-no-hana-1", "m1.no-hana", "project-A", "host1", 8192, 4, map[string]string{"trait:CUSTOM_HANA": "forbidden"}), }, FlavorRequirements: map[string]int{"m1.no-hana": 1}, @@ -411,7 +411,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host3", 16, 32, 0, 0, nil, []string{"CUSTOM_HANA"}), // HANA host for failover newHypervisor("host4", 16, 32, 0, 0, nil, nil), // Non-HANA host for failover }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithExtraSpecs("vm-hana-1", "m1.hana", "project-A", "host1", 8192, 4, map[string]string{"trait:CUSTOM_HANA": "required"}), newVMWithExtraSpecs("vm-no-hana-1", "m1.no-hana", "project-A", "host2", 8192, 4, map[string]string{"trait:CUSTOM_HANA": "forbidden"}), }, @@ -427,7 +427,7 @@ func TestIntegration(t *testing.T) { newHypervisor("host3", 16, 32, 0, 0, nil, []string{"CUSTOM_HANA"}), // Empty HANA host for failover newHypervisor("host4", 16, 32, 0, 0, nil, nil), // Non-HANA host (not usable for HANA VMs) }, - VMs: []VM{ + VMs: []reservations.VM{ newVMWithExtraSpecs("vm-hana-1", "m1.hana", "project-A", "host1", 8192, 4, map[string]string{"trait:CUSTOM_HANA": "required"}), newVMWithExtraSpecs("vm-hana-2", "m1.hana", "project-A", "host2", 8192, 4, map[string]string{"trait:CUSTOM_HANA": "required"}), }, @@ -529,7 +529,7 @@ type IntegrationTestEnv struct { K8sClient client.Client Server *httptest.Server NovaController *nova.FilterWeigherPipelineController - VMSource VMSource + VMSource reservations.VMSource SchedulerBaseURL string } @@ -569,7 +569,7 @@ func (env *IntegrationTestEnv) SendPlacementRequest(req novaapi.ExternalSchedule } // ListVMs returns all VMs from the VMSource. -func (env *IntegrationTestEnv) ListVMs() []VM { +func (env *IntegrationTestEnv) ListVMs() []reservations.VM { vms, err := env.VMSource.ListVMs(context.Background()) if err != nil { env.T.Fatalf("Failed to list VMs: %v", err) @@ -603,7 +603,7 @@ func (env *IntegrationTestEnv) LogStateSummary() { vms := env.ListVMs() reservationsList := env.ListReservations() - vmsByHypervisor := make(map[string][]VM) + vmsByHypervisor := make(map[string][]reservations.VM) for _, vm := range vms { vmsByHypervisor[vm.CurrentHypervisor] = append(vmsByHypervisor[vm.CurrentHypervisor], vm) } @@ -907,7 +907,7 @@ func (env *IntegrationTestEnv) simulateHostFailure(failedHosts, allHosts []strin } } - affectedVMs := make([]VM, 0) + affectedVMs := make([]reservations.VM, 0) for _, vm := range vms { if failedHostSet[vm.CurrentHypervisor] { affectedVMs = append(affectedVMs, vm) @@ -1038,28 +1038,39 @@ func getSharedMonitor() lib.FilterWeigherPipelineMonitor { // MockVMSource implements VMSource for testing without requiring a database. type MockVMSource struct { - VMs []VM + VMs []reservations.VM } // NewMockVMSource creates a new MockVMSource with the given VMs. -func NewMockVMSource(vms []VM) *MockVMSource { +func NewMockVMSource(vms []reservations.VM) *MockVMSource { return &MockVMSource{VMs: vms} } +// ListVMsByProject returns VMs filtered by project ID. +func (s *MockVMSource) ListVMsByProject(_ context.Context, projectID string) ([]reservations.VM, error) { + var result []reservations.VM + for _, vm := range s.VMs { + if vm.ProjectID == projectID { + result = append(result, vm) + } + } + return result, nil +} + // ListVMs returns the configured VMs. -func (s *MockVMSource) ListVMs(_ context.Context) ([]VM, error) { +func (s *MockVMSource) ListVMs(_ context.Context) ([]reservations.VM, error) { return s.VMs, nil } // ListVMsOnHypervisors returns VMs that are on the given hypervisors. // For the mock, this simply returns all VMs (filtering is not needed for tests). -func (s *MockVMSource) ListVMsOnHypervisors(_ context.Context, _ *hv1.HypervisorList, _ bool) ([]VM, error) { +func (s *MockVMSource) ListVMsOnHypervisors(_ context.Context, _ *hv1.HypervisorList, _ bool) ([]reservations.VM, error) { return s.VMs, nil } // GetVM returns a specific VM by UUID. // Returns nil, nil if the VM is not found. -func (s *MockVMSource) GetVM(_ context.Context, vmUUID string) (*VM, error) { +func (s *MockVMSource) GetVM(_ context.Context, vmUUID string) (*reservations.VM, error) { for i := range s.VMs { if s.VMs[i].UUID == vmUUID { return &s.VMs[i], nil @@ -1079,12 +1090,12 @@ func (s *MockVMSource) IsServerActive(_ context.Context, vmUUID string) (bool, e } // GetDeletedVMInfo returns nil, nil (no deleted VMs in mock). -func (s *MockVMSource) GetDeletedVMInfo(_ context.Context, _ string) (*DeletedVMInfo, error) { +func (s *MockVMSource) GetDeletedVMInfo(_ context.Context, _ string) (*reservations.DeletedVMInfo, error) { return nil, nil } // newIntegrationTestEnv creates a complete test environment with HTTP server and VMSource. -func newIntegrationTestEnv(t *testing.T, vms []VM, hypervisors []*hv1.Hypervisor, reservations []*v1alpha1.Reservation) *IntegrationTestEnv { +func newIntegrationTestEnv(t *testing.T, vms []reservations.VM, hypervisors []*hv1.Hypervisor, reservations []*v1alpha1.Reservation) *IntegrationTestEnv { t.Helper() // Combine hypervisors and reservations into a single objects slice @@ -1274,7 +1285,7 @@ func (api *testHTTPAPI) NovaExternalScheduler(w http.ResponseWriter, r *http.Req } // newIntegrationTestEnvWithTraitsFilter creates a test environment with the filter_has_requested_traits filter enabled. -func newIntegrationTestEnvWithTraitsFilter(t *testing.T, vms []VM, hypervisors []*hv1.Hypervisor, reservations []*v1alpha1.Reservation) *IntegrationTestEnv { +func newIntegrationTestEnvWithTraitsFilter(t *testing.T, vms []reservations.VM, hypervisors []*hv1.Hypervisor, reservations []*v1alpha1.Reservation) *IntegrationTestEnv { t.Helper() // Combine hypervisors and reservations into a single objects slice @@ -1477,13 +1488,13 @@ func newReservation(name, host string, memoryMB, vcpus uint64, allocations map[s // newVM creates a VM with the given parameters. // Uses defaultTestAZ as the availability zone. -func newVM(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64) VM { //nolint:unparam +func newVM(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64) reservations.VM { //nolint:unparam return newVMWithAZ(uuid, flavorName, projectID, host, memoryMB, vcpus, defaultTestAZ) } // newVMWithAZ creates a VM with the given parameters including availability zone. -func newVMWithAZ(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64, az string) VM { - return VM{ +func newVMWithAZ(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64, az string) reservations.VM { + return reservations.VM{ UUID: uuid, FlavorName: flavorName, ProjectID: projectID, @@ -1499,12 +1510,12 @@ func newVMWithAZ(uuid, flavorName, projectID, host string, memoryMB, vcpus uint6 // newVMWithExtraSpecs creates a VM with the given parameters including extra specs. // Uses defaultTestAZ as the availability zone. -func newVMWithExtraSpecs(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64, extraSpecs map[string]string) VM { //nolint:unparam +func newVMWithExtraSpecs(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64, extraSpecs map[string]string) reservations.VM { //nolint:unparam return newVMWithExtraSpecsAndAZ(uuid, flavorName, projectID, host, memoryMB, vcpus, extraSpecs, defaultTestAZ) } // newVMWithExtraSpecsAndAZ creates a VM with the given parameters including extra specs and availability zone. -func newVMWithExtraSpecsAndAZ(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64, extraSpecs map[string]string, az string) VM { +func newVMWithExtraSpecsAndAZ(uuid, flavorName, projectID, host string, memoryMB, vcpus uint64, extraSpecs map[string]string, az string) reservations.VM { vm := newVMWithAZ(uuid, flavorName, projectID, host, memoryMB, vcpus, az) if extraSpecs != nil { vm.FlavorExtraSpecs = extraSpecs diff --git a/internal/scheduling/reservations/failover/reservation_eligibility.go b/internal/scheduling/reservations/failover/reservation_eligibility.go index 1f6e7353f..abcaa636c 100644 --- a/internal/scheduling/reservations/failover/reservation_eligibility.go +++ b/internal/scheduling/reservations/failover/reservation_eligibility.go @@ -7,6 +7,7 @@ import ( "slices" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) @@ -36,7 +37,7 @@ type DependencyGraph struct { // (3) For any reservation r, no two VMs that use r may be on the same hypervisor. // (4) For VM v with slots R, any other VM that uses any slot must not run on v's host or slot hosts. // (5) For VM v with slots R, no two other VMs using v's slots can be on the same hypervisor. -func IsVMEligibleForReservation(vm VM, reservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation) bool { +func IsVMEligibleForReservation(vm reservations.VM, reservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation) bool { // Check if VM is already using this reservation resAllocations := getFailoverAllocations(&reservation) if _, exists := resAllocations[vm.UUID]; exists { @@ -62,7 +63,7 @@ func IsVMEligibleForReservation(vm VM, reservation v1alpha1.Reservation, allFail // CheckVMsStillEligible checks if VMs in reservations are still eligible. // Returns a map of reservation name -> list of VM UUIDs that are no longer eligible. func CheckVMsStillEligible( - vms map[string]VM, + vms map[string]reservations.VM, failoverReservations []v1alpha1.Reservation, ) map[string][]string { @@ -103,7 +104,7 @@ func CheckVMsStillEligible( // FindEligibleReservations finds all reservations that a VM is eligible to use. func FindEligibleReservations( - vm VM, + vm reservations.VM, failoverReservations []v1alpha1.Reservation, ) []v1alpha1.Reservation { @@ -172,7 +173,7 @@ func newBaseDependencyGraph(allFailoverReservations []v1alpha1.Reservation) *Dep // newDependencyGraph builds a DependencyGraph with the VM added to the candidate reservation. func newDependencyGraph( - vm VM, + vm reservations.VM, candidateReservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation, ) *DependencyGraph { @@ -289,7 +290,7 @@ func (g *DependencyGraph) isVMEligibleForReservation(candidateResName string) bo } // doesVMFitInReservation checks if a VM's resources fit within a reservation's resources. -func doesVMFitInReservation(vm VM, reservation v1alpha1.Reservation) bool { +func doesVMFitInReservation(vm reservations.VM, reservation v1alpha1.Reservation) bool { if vmMemory, ok := vm.Resources["memory"]; ok { if resMemory, ok := reservation.Spec.Resources[hv1.ResourceMemory]; ok { if vmMemory.Cmp(resMemory) > 0 { diff --git a/internal/scheduling/reservations/failover/reservation_eligibility_test.go b/internal/scheduling/reservations/failover/reservation_eligibility_test.go index 841d15abe..d66d3322e 100644 --- a/internal/scheduling/reservations/failover/reservation_eligibility_test.go +++ b/internal/scheduling/reservations/failover/reservation_eligibility_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -61,8 +62,8 @@ func makeReservationWithResources(name, host string, usedBy map[string]string, r } // makeVM creates a test VM with the given parameters. -func makeVM(uuid, hypervisor string) VM { - return VM{ +func makeVM(uuid, hypervisor string) reservations.VM { + return reservations.VM{ UUID: uuid, CurrentHypervisor: hypervisor, Resources: defaultVMResources, @@ -70,8 +71,8 @@ func makeVM(uuid, hypervisor string) VM { } // makeVMWithResources creates a test VM with custom resources. -func makeVMWithResources(uuid, hypervisor string, resources map[string]resource.Quantity) VM { //nolint:unparam // uuid is always "vm-1" in tests but kept for clarity - return VM{ +func makeVMWithResources(uuid, hypervisor string, resources map[string]resource.Quantity) reservations.VM { //nolint:unparam // uuid is always "vm-1" in tests but kept for clarity + return reservations.VM{ UUID: uuid, CurrentHypervisor: hypervisor, Resources: resources, @@ -82,7 +83,7 @@ func makeVMWithResources(uuid, hypervisor string, resources map[string]resource. // It also includes the VM we are checking (vm) with its current hypervisor, // and the candidate reservation (which may have VMs not in allFailoverReservations). // This is a test helper function used to verify data structure consistency. -func buildVMHypervisorsMap(vm VM, candidateReservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation) map[string]map[string]bool { +func buildVMHypervisorsMap(vm reservations.VM, candidateReservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation) map[string]map[string]bool { vmHypervisorsMap := make(map[string]map[string]bool) vmHypervisorsMap[vm.UUID] = make(map[string]bool) @@ -115,7 +116,7 @@ func buildVMHypervisorsMap(vm VM, candidateReservation v1alpha1.Reservation, all func TestIsVMEligibleForReservation(t *testing.T) { testCases := []struct { name string - vm VM + vm reservations.VM reservation v1alpha1.Reservation vmHostMap map[string]string allReservations []v1alpha1.Reservation @@ -614,7 +615,7 @@ func TestIsVMEligibleForReservation(t *testing.T) { func TestDoesVMFitInReservation(t *testing.T) { testCases := []struct { name string - vm VM + vm reservations.VM reservation v1alpha1.Reservation expected bool }{ @@ -727,7 +728,7 @@ func updateReservationInList(reservations []v1alpha1.Reservation, updated v1alph // all existing VMs in that reservation remain eligible. // Returns (allEligible, failedVMUUID, reason). func checkAllExistingVMsRemainEligible( - newVM VM, + newVM reservations.VM, reservation v1alpha1.Reservation, allReservations []v1alpha1.Reservation, ) (allEligible bool, failedVMUUID, reason string) { @@ -752,7 +753,7 @@ func checkAllExistingVMsRemainEligible( // Check each existing VM in the reservation for vmUUID, vmHost := range existingAllocations { - existingVM := VM{UUID: vmUUID, CurrentHypervisor: vmHost, Resources: defaultVMResources} + existingVM := reservations.VM{UUID: vmUUID, CurrentHypervisor: vmHost, Resources: defaultVMResources} // Temporarily remove the VM to check if it can be "re-added" // This mimics what reconcileRemoveNoneligibleVMFromReservations does @@ -774,7 +775,7 @@ func checkAllExistingVMsRemainEligible( func TestAddingVMDoesNotMakeOthersIneligible(t *testing.T) { testCases := []struct { name string - vm VM + vm reservations.VM reservation v1alpha1.Reservation allReservations []v1alpha1.Reservation vmIsEligible bool // Expected result of IsVMEligibleForReservation @@ -1142,8 +1143,8 @@ func TestAddingVMDoesNotMakeOthersIneligible(t *testing.T) { func TestSymmetryOfEligibility(t *testing.T) { testCases := []struct { name string - vm1 VM - vm2 VM + vm1 reservations.VM + vm2 reservations.VM // vm1Reservation is the reservation to check for vm1's eligibility vm1Reservation v1alpha1.Reservation // vm2Reservation is the reservation to check for vm2's eligibility @@ -1254,7 +1255,7 @@ func TestDataStructureConsistency(t *testing.T) { testCases := []struct { name string - vm VM + vm reservations.VM reservation v1alpha1.Reservation allReservations []v1alpha1.Reservation }{ @@ -1578,7 +1579,7 @@ func graphsEqual(t *testing.T, actual, expected *DependencyGraph) { func TestNewDependencyGraph(t *testing.T) { testCases := []struct { name string - vm VM + vm reservations.VM candidateRes v1alpha1.Reservation allReservations []v1alpha1.Reservation expectGraph *DependencyGraph @@ -1890,7 +1891,7 @@ func TestAddRemoveVMRoundTrip(t *testing.T) { func TestFindEligibleReservations(t *testing.T) { testCases := []struct { name string - vm VM + vm reservations.VM failoverReservations []v1alpha1.Reservation vmHostMap map[string]string expectedCount int diff --git a/internal/scheduling/reservations/failover/reservation_scheduling.go b/internal/scheduling/reservations/failover/reservation_scheduling.go index f482f3393..209c0748f 100644 --- a/internal/scheduling/reservations/failover/reservation_scheduling.go +++ b/internal/scheduling/reservations/failover/reservation_scheduling.go @@ -30,7 +30,7 @@ const ( PipelineAcknowledgeFailoverReservation = "kvm-acknowledge-failover-reservation" ) -func (c *FailoverReservationController) queryHypervisorsFromScheduler(ctx context.Context, vm VM, allHypervisors []string, pipeline string, resSpec resolvedReservationSpec) ([]string, error) { +func (c *FailoverReservationController) queryHypervisorsFromScheduler(ctx context.Context, vm reservations.VM, allHypervisors []string, pipeline string, resSpec resolvedReservationSpec) ([]string, error) { logger := LoggerFromContext(ctx) // Build list of eligible hypervisors (excluding VM's current hypervisor) @@ -113,7 +113,7 @@ func (c *FailoverReservationController) queryHypervisorsFromScheduler(ctx contex // The caller is responsible for persisting the changes to the cluster. func (c *FailoverReservationController) tryReuseExistingReservation( ctx context.Context, - vm VM, + vm reservations.VM, failoverReservations []v1alpha1.Reservation, allHypervisors []string, resSpec resolvedReservationSpec, @@ -172,7 +172,7 @@ func (c *FailoverReservationController) tryReuseExistingReservation( // TODO this is a bit of a hack. Ideally we have a special kind of request for that which would also verify that we equally are using the reservation func (c *FailoverReservationController) validateVMViaSchedulerEvacuation( ctx context.Context, - vm VM, + vm reservations.VM, reservationHost string, ) (bool, error) { @@ -253,7 +253,7 @@ func (c *FailoverReservationController) validateVMViaSchedulerEvacuation( // that already had a new reservation created in this reconcile cycle). func (c *FailoverReservationController) scheduleAndBuildNewFailoverReservation( ctx context.Context, - vm VM, + vm reservations.VM, allHypervisors []string, failoverReservations []v1alpha1.Reservation, excludeHypervisors map[string]bool, diff --git a/internal/scheduling/reservations/failover/reservation_scheduling_test.go b/internal/scheduling/reservations/failover/reservation_scheduling_test.go index fa987d34b..c80ef4bf9 100644 --- a/internal/scheduling/reservations/failover/reservation_scheduling_test.go +++ b/internal/scheduling/reservations/failover/reservation_scheduling_test.go @@ -9,6 +9,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -22,7 +23,7 @@ func TestBuildReservationWithVM(t *testing.T) { tests := []struct { name string reservation v1alpha1.Reservation - vm VM + vm reservations.VM wantVMInAllocations bool wantAllocationsCount int wantOriginalUnmodified bool @@ -107,7 +108,7 @@ func TestBuildReservationWithVM(t *testing.T) { func TestBuildNewFailoverReservation(t *testing.T) { tests := []struct { name string - vm VM + vm reservations.VM hypervisor string wantHost string wantTargetHost string @@ -250,7 +251,7 @@ func TestResolveVMForSchedulingAndNewFailoverReservation(t *testing.T) { tests := []struct { name string - vm VM + vm reservations.VM useFlavorGroupResources bool flavorGroups map[string]compute.FlavorGroupFeature wantFlavorName string @@ -261,7 +262,7 @@ func TestResolveVMForSchedulingAndNewFailoverReservation(t *testing.T) { }{ { name: "uses LargestFlavor resources when enabled and flavor found", - vm: VM{ + vm: reservations.VM{ UUID: "vm-1", CurrentHypervisor: "host1", FlavorName: "hana_c60_m960", @@ -281,7 +282,7 @@ func TestResolveVMForSchedulingAndNewFailoverReservation(t *testing.T) { }, { name: "falls back to VM resources when disabled", - vm: VM{ + vm: reservations.VM{ UUID: "vm-2", CurrentHypervisor: "host1", FlavorName: "hana_c60_m960", @@ -301,7 +302,7 @@ func TestResolveVMForSchedulingAndNewFailoverReservation(t *testing.T) { }, { name: "falls back to VM resources when flavor not in any group", - vm: VM{ + vm: reservations.VM{ UUID: "vm-3", CurrentHypervisor: "host1", FlavorName: "unknown_flavor", @@ -321,7 +322,7 @@ func TestResolveVMForSchedulingAndNewFailoverReservation(t *testing.T) { }, { name: "falls back to VM resources when flavorGroups is nil", - vm: VM{ + vm: reservations.VM{ UUID: "vm-4", CurrentHypervisor: "host1", FlavorName: "hana_c60_m960", @@ -440,8 +441,8 @@ func buildSchedulingTestReservationNoStatus(name, host string) v1alpha1.Reservat } } -func buildSchedulingTestVM(uuid, hypervisor string) VM { //nolint:unparam // uuid may vary in future tests - return VM{ +func buildSchedulingTestVM(uuid, hypervisor string) reservations.VM { //nolint:unparam // uuid may vary in future tests + return reservations.VM{ UUID: uuid, CurrentHypervisor: hypervisor, FlavorName: "m1.large", @@ -453,8 +454,8 @@ func buildSchedulingTestVM(uuid, hypervisor string) VM { //nolint:unparam // uui } } -func buildSchedulingTestVMWithResources(uuid, hypervisor string, memoryMB, vcpus int64) VM { - return VM{ +func buildSchedulingTestVMWithResources(uuid, hypervisor string, memoryMB, vcpus int64) reservations.VM { + return reservations.VM{ UUID: uuid, CurrentHypervisor: hypervisor, FlavorName: "m1.large", diff --git a/internal/scheduling/reservations/hypervisor_diff_handler.go b/internal/scheduling/reservations/hypervisor_diff_handler.go new file mode 100644 index 000000000..5ef6b42b1 --- /dev/null +++ b/internal/scheduling/reservations/hypervisor_diff_handler.go @@ -0,0 +1,36 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package reservations + +import ( + "context" + + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + "k8s.io/client-go/util/workqueue" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// HypervisorDiffHandler is a typed event handler that calls a callback on HV Update events. +// Create and Delete events are no-ops; a periodic full reconcile is expected to correct any drift. +type HypervisorDiffHandler struct { + // OnUpdate is called with (ctx, oldHV, newHV) on every Hypervisor Update event. + OnUpdate func(ctx context.Context, oldHV, newHV *hv1.Hypervisor) error +} + +func (h *HypervisorDiffHandler) Create(_ context.Context, _ event.TypedCreateEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { +} + +func (h *HypervisorDiffHandler) Update(ctx context.Context, e event.TypedUpdateEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { + if err := h.OnUpdate(ctx, e.ObjectOld, e.ObjectNew); err != nil { + ctrl.Log.WithName("hypervisor-diff-handler").Error(err, "failed to process HV diff", "hypervisor", e.ObjectNew.Name) + } +} + +func (h *HypervisorDiffHandler) Delete(_ context.Context, _ event.TypedDeleteEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { +} + +func (h *HypervisorDiffHandler) Generic(_ context.Context, _ event.TypedGenericEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { +} diff --git a/internal/scheduling/reservations/quota/controller.go b/internal/scheduling/reservations/quota/controller.go index 186ae50b5..122898b0b 100644 --- a/internal/scheduling/reservations/quota/controller.go +++ b/internal/scheduling/reservations/quota/controller.go @@ -14,13 +14,11 @@ import ( "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" commitments "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/commitments" - "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/failover" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/util/retry" - "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,7 +39,7 @@ var log = ctrl.Log.WithName("quota-controller").WithValues("module", "quota-hand // - PaygUsage-only recompute: triggered by CR or ProjectQuota spec changes type QuotaController struct { client.Client - VMSource failover.VMSource + VMSource reservations.VMSource Config QuotaControllerConfig Metrics *QuotaMetrics } @@ -49,7 +47,7 @@ type QuotaController struct { // NewQuotaController creates a new QuotaController. func NewQuotaController( c client.Client, - vmSource failover.VMSource, + vmSource reservations.VMSource, config QuotaControllerConfig, metrics *QuotaMetrics, ) *QuotaController { @@ -465,7 +463,7 @@ func (c *QuotaController) accumulateAddedVM( // missed it. In that case we would also skip the increment here (CreatedAt <= LastReconcileAt) // and the VM would only be counted on the NEXT full reconcile cycle. This is acceptable for // now and will be resolved when we move to a CRD-based VM source with real-time events. -func (c *QuotaController) isVMNewSinceLastReconcile(ctx context.Context, vm *failover.VM) bool { +func (c *QuotaController) isVMNewSinceLastReconcile(ctx context.Context, vm *reservations.VM) bool { if vm.CreatedAt == "" { // No creation time available -- be conservative, skip increment. // The next full reconcile will pick it up. @@ -684,7 +682,11 @@ func (c *QuotaController) SetupHVWatcher(mgr ctrl.Manager) error { WatchesRawSource(source.Kind( mgr.GetCache(), &hv1.Hypervisor{}, - &hvInstanceDiffHandler{controller: c}, + &reservations.HypervisorDiffHandler{ + OnUpdate: func(ctx context.Context, oldHV, newHV *hv1.Hypervisor) error { + return c.ReconcileHVDiff(ctx, oldHV, newHV) + }, + }, hvInstanceChangePredicate(), )). WithOptions(controller.Options{ @@ -737,7 +739,7 @@ func (c *QuotaController) Start(ctx context.Context) error { // This matches the unit system used by LIQUID for commitment tracking. // The per-AZ breakdown allows Limes to enforce AZ-level quota limits. func (c *QuotaController) computeTotalUsage( - vms []failover.VM, + vms []reservations.VM, flavorToGroup map[string]string, flavorGroups map[string]compute.FlavorGroupFeature, ) map[string]map[string]map[string]int64 { @@ -1221,26 +1223,3 @@ func hvInstanceChangePredicate() predicate.TypedPredicate[*hv1.Hypervisor] { GenericFunc: func(_ event.TypedGenericEvent[*hv1.Hypervisor]) bool { return false }, } } - -// hvInstanceDiffHandler handles HV instance diff events by calling ReconcileHVDiff. -type hvInstanceDiffHandler struct { - controller *QuotaController -} - -func (h *hvInstanceDiffHandler) Create(_ context.Context, _ event.TypedCreateEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { - // On create, no diff needed (full reconcile will catch up) -} - -func (h *hvInstanceDiffHandler) Update(ctx context.Context, e event.TypedUpdateEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { - if err := h.controller.ReconcileHVDiff(ctx, e.ObjectOld, e.ObjectNew); err != nil { - log.Error(err, "failed to process HV instance diff", "hypervisor", e.ObjectNew.Name) - } -} - -func (h *hvInstanceDiffHandler) Delete(_ context.Context, _ event.TypedDeleteEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { - // On delete, full reconcile will correct -} - -func (h *hvInstanceDiffHandler) Generic(_ context.Context, _ event.TypedGenericEvent[*hv1.Hypervisor], _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { - // No-op -} diff --git a/internal/scheduling/reservations/quota/controller_test.go b/internal/scheduling/reservations/quota/controller_test.go index 2becea2db..a5e5c937b 100644 --- a/internal/scheduling/reservations/quota/controller_test.go +++ b/internal/scheduling/reservations/quota/controller_test.go @@ -10,7 +10,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/failover" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -38,7 +38,7 @@ func TestComputeTotalUsage(t *testing.T) { } flavorToGroup := buildFlavorToGroupMap(flavorGroups) - vms := []failover.VM{ + vms := []reservations.VM{ { UUID: "vm-1", FlavorName: "m1.hana_v2.small", @@ -479,8 +479,8 @@ func TestAccumulateAddedVM_UnknownFlavor(t *testing.T) { // Use a mock VMSource that returns a VM with unknown flavor ctrl.VMSource = &mockVMSource{ - getVM: func(_ context.Context, vmUUID string) (*failover.VM, error) { - return &failover.VM{ + getVM: func(_ context.Context, vmUUID string) (*reservations.VM, error) { + return &reservations.VM{ UUID: vmUUID, FlavorName: "unknown-flavor", ProjectID: "project-a", @@ -542,8 +542,8 @@ func TestAccumulateAddedVM_KnownFlavor(t *testing.T) { projectDeltas := make(map[string]*usageDelta) qc.VMSource = &mockVMSource{ - getVM: func(_ context.Context, vmUUID string) (*failover.VM, error) { - return &failover.VM{ + getVM: func(_ context.Context, vmUUID string) (*reservations.VM, error) { + return &reservations.VM{ UUID: vmUUID, FlavorName: "m1.hana_v2.small", ProjectID: "project-a", @@ -578,27 +578,31 @@ func TestAccumulateAddedVM_KnownFlavor(t *testing.T) { // mockVMSource is a test helper for VMSource. type mockVMSource struct { - listVMs func(ctx context.Context) ([]failover.VM, error) - getVM func(ctx context.Context, vmUUID string) (*failover.VM, error) + listVMs func(ctx context.Context) ([]reservations.VM, error) + getVM func(ctx context.Context, vmUUID string) (*reservations.VM, error) isServerActive func(ctx context.Context, vmUUID string) (bool, error) - getDeletedVM func(ctx context.Context, vmUUID string) (*failover.DeletedVMInfo, error) + getDeletedVM func(ctx context.Context, vmUUID string) (*reservations.DeletedVMInfo, error) } -func (m *mockVMSource) ListVMs(ctx context.Context) ([]failover.VM, error) { +func (m *mockVMSource) ListVMsByProject(_ context.Context, _ string) ([]reservations.VM, error) { + return nil, nil +} + +func (m *mockVMSource) ListVMs(ctx context.Context) ([]reservations.VM, error) { if m.listVMs != nil { return m.listVMs(ctx) } return nil, nil } -func (m *mockVMSource) GetVM(ctx context.Context, vmUUID string) (*failover.VM, error) { +func (m *mockVMSource) GetVM(ctx context.Context, vmUUID string) (*reservations.VM, error) { if m.getVM != nil { return m.getVM(ctx, vmUUID) } return nil, nil } -func (m *mockVMSource) ListVMsOnHypervisors(_ context.Context, _ *hv1.HypervisorList, _ bool) ([]failover.VM, error) { +func (m *mockVMSource) ListVMsOnHypervisors(_ context.Context, _ *hv1.HypervisorList, _ bool) ([]reservations.VM, error) { return nil, nil } @@ -609,7 +613,7 @@ func (m *mockVMSource) IsServerActive(ctx context.Context, vmUUID string) (bool, return false, nil } -func (m *mockVMSource) GetDeletedVMInfo(ctx context.Context, vmUUID string) (*failover.DeletedVMInfo, error) { +func (m *mockVMSource) GetDeletedVMInfo(ctx context.Context, vmUUID string) (*reservations.DeletedVMInfo, error) { if m.getDeletedVM != nil { return m.getDeletedVM(ctx, vmUUID) } diff --git a/internal/scheduling/reservations/quota/integration_test.go b/internal/scheduling/reservations/quota/integration_test.go index d960ab396..9f55a25f6 100644 --- a/internal/scheduling/reservations/quota/integration_test.go +++ b/internal/scheduling/reservations/quota/integration_test.go @@ -11,7 +11,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/failover" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -161,7 +161,7 @@ func TestIntegration(t *testing.T) { activeInstance("vm-new"), // new instance }), OverrideVMs: withExtraVMs( - failover.VM{ + reservations.VM{ UUID: "vm-new", FlavorName: "m1.hana_v2.small", ProjectID: "project-a", AvailabilityZone: "az-1", CreatedAt: "2099-01-01T00:00:00Z", // far future, always AFTER last reconcile @@ -235,7 +235,7 @@ func TestIntegration(t *testing.T) { FlavorGroups: testFlavorGroups, VMs: testVMs, // vm-del is not in VMs (deleted), but has info in DeletedVMs - DeletedVMs: map[string]*failover.DeletedVMInfo{ + DeletedVMs: map[string]*reservations.DeletedVMInfo{ "vm-del": { ProjectID: "project-a", FlavorName: "m1.hana_v2.small", @@ -392,7 +392,7 @@ func TestIntegration(t *testing.T) { { Name: "unknown flavor VMs are skipped", FlavorGroups: testFlavorGroups, - VMs: []failover.VM{ + VMs: []reservations.VM{ { UUID: "vm-unknown", FlavorName: "nonexistent-flavor", ProjectID: "project-x", AvailabilityZone: "az-1", @@ -535,7 +535,7 @@ func TestIntegration(t *testing.T) { activeInstance("vm-phantom"), }), OverrideVMs: withExtraVMs( - failover.VM{ + reservations.VM{ UUID: "vm-phantom", FlavorName: "m1.hana_v2.small", ProjectID: "project-a", AvailabilityZone: "az-1", CreatedAt: "2099-01-01T00:00:00Z", // after last reconcile @@ -579,7 +579,7 @@ func TestIntegration(t *testing.T) { Name: "complex multi-project scenario with adds, removes, and reconcile corrections", FlavorGroups: testFlavorGroups, VMs: testVMs, - DeletedVMs: map[string]*failover.DeletedVMInfo{ + DeletedVMs: map[string]*reservations.DeletedVMInfo{ "vm-del": { ProjectID: "project-a", FlavorName: "m1.hana_v2.small", @@ -633,7 +633,7 @@ func TestIntegration(t *testing.T) { activeInstance("vm-new-a"), }), OverrideVMs: withExtraVMs( - failover.VM{ + reservations.VM{ UUID: "vm-new-a", FlavorName: "m1.hana_v2.small", ProjectID: "project-a", AvailabilityZone: "az-1", CreatedAt: "2099-01-01T00:00:00Z", @@ -666,7 +666,7 @@ func TestIntegration(t *testing.T) { activeInstance("vm-phantom-b"), }), OverrideVMs: withExtraVMs( - failover.VM{ + reservations.VM{ UUID: "vm-phantom-b", FlavorName: "m1.general.small", ProjectID: "project-b", AvailabilityZone: "az-1", CreatedAt: "2099-01-01T00:00:00Z", @@ -701,7 +701,7 @@ func TestIntegration(t *testing.T) { }), OverrideVMs: withExtraVMs( - failover.VM{ + reservations.VM{ UUID: "vm-new-a", FlavorName: "m1.hana_v2.small", ProjectID: "project-a", AvailabilityZone: "az-1", CreatedAt: "2099-01-01T00:00:00Z", @@ -729,7 +729,7 @@ func TestIntegration(t *testing.T) { // - project-b: FIXES drift -- truth is 1, delta said 2 (phantom gone) { Type: "full_reconcile", - OverrideVMs: &[]failover.VM{ + OverrideVMs: &[]reservations.VM{ // testVMs + vm-new-a testVMs[0], testVMs[1], testVMs[2], testVMs[3], testVMs[4], { @@ -772,7 +772,7 @@ func TestIntegration(t *testing.T) { activeInstance("vm-new-a"), }), OverrideVMs: withExtraVMs( - failover.VM{ + reservations.VM{ UUID: "vm-new-a", FlavorName: "m1.hana_v2.small", ProjectID: "project-a", AvailabilityZone: "az-1", CreatedAt: "2099-01-01T00:00:00Z", @@ -953,7 +953,7 @@ var testFlavorGroups = map[string]compute.FlavorGroupFeature{ // Standard VM set for most tests. // project-a has VMs in BOTH flavor groups (hana_v2 and general). // project-b has only general VMs. -var testVMs = []failover.VM{ +var testVMs = []reservations.VM{ // vm-1: hana_v2, 32 GiB RAM (32768/1024), 8 cores { UUID: "vm-1", FlavorName: "m1.hana_v2.small", @@ -1026,7 +1026,7 @@ type TestAction struct { // THIS action and all subsequent actions. Use to simulate VMs appearing or // disappearing between steps. To "undo" a temporary VM, set OverrideVMs // again in a later action without that VM. - OverrideVMs *[]failover.VM + OverrideVMs *[]reservations.VM // For cr_update actions: CRName string @@ -1043,9 +1043,9 @@ type IntegrationTestCase struct { Name string // Initial state seeded into the fake client and mock VMSource - VMs []failover.VM - DeletedVMs map[string]*failover.DeletedVMInfo // UUID -> deleted VM info - ActiveVMs map[string]bool // UUID -> IsServerActive response + VMs []reservations.VM + DeletedVMs map[string]*reservations.DeletedVMInfo // UUID -> deleted VM info + ActiveVMs map[string]bool // UUID -> IsServerActive response FlavorGroups map[string]compute.FlavorGroupFeature ProjectQuotas []*v1alpha1.ProjectQuota @@ -1103,10 +1103,10 @@ func newIntegrationTestEnv(t *testing.T, tc IntegrationTestCase) *integrationTes // Build mock VMSource vmSrc := &mockVMSource{ - listVMs: func(_ context.Context) ([]failover.VM, error) { + listVMs: func(_ context.Context) ([]reservations.VM, error) { return tc.VMs, nil }, - getVM: func(_ context.Context, vmUUID string) (*failover.VM, error) { + getVM: func(_ context.Context, vmUUID string) (*reservations.VM, error) { for i := range tc.VMs { if tc.VMs[i].UUID == vmUUID { return &tc.VMs[i], nil @@ -1122,7 +1122,7 @@ func newIntegrationTestEnv(t *testing.T, tc IntegrationTestCase) *integrationTes } return false, nil }, - getDeletedVM: func(_ context.Context, vmUUID string) (*failover.DeletedVMInfo, error) { + getDeletedVM: func(_ context.Context, vmUUID string) (*reservations.DeletedVMInfo, error) { if tc.DeletedVMs != nil { if info, ok := tc.DeletedVMs[vmUUID]; ok { return info, nil @@ -1275,10 +1275,10 @@ func (env *integrationTestEnv) executeAction(action TestAction) { // Apply OverrideVMs if set (persists for all subsequent actions) if action.OverrideVMs != nil { vms := *action.OverrideVMs - env.vmSource.listVMs = func(_ context.Context) ([]failover.VM, error) { + env.vmSource.listVMs = func(_ context.Context) ([]reservations.VM, error) { return vms, nil } - env.vmSource.getVM = func(_ context.Context, vmUUID string) (*failover.VM, error) { + env.vmSource.getVM = func(_ context.Context, vmUUID string) (*reservations.VM, error) { for i := range vms { if vms[i].UUID == vmUUID { return &vms[i], nil @@ -1461,14 +1461,14 @@ func int64Ptr(v int64) *int64 { return &v } // withExtraVMs returns a pointer to testVMs + additional VMs. // Used with OverrideVMs to add VMs to the "world" for an action. -func withExtraVMs(extra ...failover.VM) *[]failover.VM { - vms := append(append([]failover.VM{}, testVMs...), extra...) +func withExtraVMs(extra ...reservations.VM) *[]reservations.VM { + vms := append(append([]reservations.VM{}, testVMs...), extra...) return &vms } // baseVMsPtr returns a pointer to a copy of testVMs (resets to baseline). -func baseVMsPtr() *[]failover.VM { - vms := append([]failover.VM{}, testVMs...) +func baseVMsPtr() *[]reservations.VM { + vms := append([]reservations.VM{}, testVMs...) return &vms } diff --git a/internal/scheduling/reservations/failover/vm_source.go b/internal/scheduling/reservations/vm_source.go similarity index 61% rename from internal/scheduling/reservations/failover/vm_source.go rename to internal/scheduling/reservations/vm_source.go index 5a9603b79..ab5998b62 100644 --- a/internal/scheduling/reservations/failover/vm_source.go +++ b/internal/scheduling/reservations/vm_source.go @@ -1,22 +1,31 @@ // Copyright SAP SE // SPDX-License-Identifier: Apache-2.0 -package failover +package reservations import ( "context" "encoding/json" "fmt" + "sync" "github.com/cobaltcore-dev/cortex/internal/scheduling/external" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" ) -// VM represents a virtual machine that may need failover reservations. +var vmSourceLog = logf.Log.WithName("vm-source") + +// VM represents a virtual machine managed by the reservation system. type VM struct { // UUID is the unique identifier of the VM. UUID string + // Name is the display name of the VM in Nova. + Name string + // Status is the Nova status of the VM (e.g. ACTIVE, SHUTOFF). + Status string // FlavorName is the name of the flavor used by the VM. FlavorName string // ProjectID is the OpenStack project ID that owns the VM. @@ -34,27 +43,32 @@ type VM struct { // FlavorExtraSpecs contains the flavor's extra specifications (e.g., traits, capabilities). // This is used by filters like filter_has_requested_traits and filter_capabilities. FlavorExtraSpecs map[string]string + // DiskGB is the flavor's root disk size in GiB. + DiskGB uint64 + // OSType is the operating system type pre-computed from Glance image properties. + // "unknown" when not found or for volume-booted instances. + OSType string } -// VMSource provides VMs that may need failover reservations. +// VMSource provides VMs managed by the reservation system. // This interface allows swapping the implementation when a VM CRD arrives. type VMSource interface { - // ListVMs returns all VMs that might need failover reservations. + // ListVMs returns all VMs across all projects. ListVMs(ctx context.Context) ([]VM, error) + // ListVMsByProject returns all VMs belonging to a specific project. + ListVMsByProject(ctx context.Context, projectID string) ([]VM, error) // ListVMsOnHypervisors returns VMs that are on the given hypervisors. // If trustHypervisorLocation is true, uses hypervisor CRD as source of truth for VM location. // If trustHypervisorLocation is false, uses postgres as source of truth but filters to VMs on known hypervisors. - // Also logs warnings about data sync issues between postgres and hypervisor CRD. ListVMsOnHypervisors(ctx context.Context, hypervisorList *hv1.HypervisorList, trustHypervisorLocation bool) ([]VM, error) // GetVM returns a specific VM by UUID. - // Returns nil, nil if the VM is not found (not an error, just doesn't exist). + // Returns nil, nil if the VM is not found. GetVM(ctx context.Context, vmUUID string) (*VM, error) - // IsServerActive returns true if the server exists in the servers table (still running somewhere). + // IsServerActive returns true if the server exists in the servers table and is not DELETED. // Returns false if not found. Used by quota controller to determine if a removed HV instance was deleted vs migrated. IsServerActive(ctx context.Context, vmUUID string) (bool, error) // GetDeletedVMInfo returns metadata about a deleted VM (from deleted_servers table), // including resolved flavor resources. Returns nil, nil if not found. - // Used by quota controller for incremental usage decrements. GetDeletedVMInfo(ctx context.Context, vmUUID string) (*DeletedVMInfo, error) } @@ -78,212 +92,270 @@ func NewDBVMSource(novaReader external.NovaReaderInterface) *DBVMSource { return &DBVMSource{NovaReader: novaReader} } -// ListVMs returns all VMs by joining server and flavor data from the database. +// ListVMs returns all VMs across all projects by joining server and flavor data. func (s *DBVMSource) ListVMs(ctx context.Context) ([]VM, error) { - // Fetch all servers servers, err := s.NovaReader.GetAllServers(ctx) if err != nil { return nil, fmt.Errorf("failed to get servers: %w", err) } - // Fetch all flavors and build a lookup map flavors, err := s.NovaReader.GetAllFlavors(ctx) if err != nil { return nil, fmt.Errorf("failed to get flavors: %w", err) } - flavorByName := make(map[string]struct { + type flavorData struct { VCPUs uint64 RAM uint64 + Disk uint64 ExtraSpecs string - }) + } + flavorByName := make(map[string]flavorData, len(flavors)) for _, f := range flavors { - flavorByName[f.Name] = struct { - VCPUs uint64 - RAM uint64 - ExtraSpecs string - }{VCPUs: f.VCPUs, RAM: f.RAM, ExtraSpecs: f.ExtraSpecs} + flavorByName[f.Name] = flavorData{VCPUs: f.VCPUs, RAM: f.RAM, Disk: f.Disk, ExtraSpecs: f.ExtraSpecs} } - // Track filtering statistics var skippedNoHost, skippedUnknownFlavor int unknownFlavors := make(map[string]int) - // Convert servers to VMs vms := make([]VM, 0, len(servers)) for _, server := range servers { - // Skip servers without a host (not yet scheduled) if server.OSEXTSRVATTRHost == "" { skippedNoHost++ continue } - - // Look up flavor resources flavor, ok := flavorByName[server.FlavorName] if !ok { - // Skip servers with unknown flavors skippedUnknownFlavor++ unknownFlavors[server.FlavorName]++ continue } - - // Build resources map resources := map[string]resource.Quantity{ - "vcpus": *resource.NewQuantity(int64(flavor.VCPUs), resource.DecimalSI), //nolint:gosec // VCPUs won't overflow int64 - "memory": *resource.NewQuantity(int64(flavor.RAM)*1024*1024, resource.BinarySI), //nolint:gosec // RAM in MB won't overflow int64 + "vcpus": *resource.NewQuantity(int64(flavor.VCPUs), resource.DecimalSI), //nolint:gosec + "memory": *resource.NewQuantity(int64(flavor.RAM)*1024*1024, resource.BinarySI), //nolint:gosec } - - // Parse extra specs from JSON string - extraSpecs := parseExtraSpecs(flavor.ExtraSpecs) - vms = append(vms, VM{ UUID: server.ID, + Name: server.Name, + Status: server.Status, FlavorName: server.FlavorName, ProjectID: server.TenantID, CurrentHypervisor: server.OSEXTSRVATTRHost, AvailabilityZone: server.OSEXTAvailabilityZone, CreatedAt: server.Created, Resources: resources, - FlavorExtraSpecs: extraSpecs, + FlavorExtraSpecs: parseExtraSpecs(flavor.ExtraSpecs), + DiskGB: flavor.Disk, + OSType: normalizeOSType(server.OSType), }) } - // Log filtering statistics at debug level (verbose) - log.V(1).Info("ListVMs filtering statistics", + vmSourceLog.V(1).Info("ListVMs filtering statistics", "totalServersInDB", len(servers), "skippedNoHost", skippedNoHost, "skippedUnknownFlavor", skippedUnknownFlavor, "totalFlavorsInDB", len(flavors), "returnedVMs", len(vms)) if len(unknownFlavors) > 0 { - log.V(1).Info("ListVMs unknown flavors", "unknownFlavors", unknownFlavors) + vmSourceLog.V(1).Info("ListVMs unknown flavors", "unknownFlavors", unknownFlavors) } return vms, nil } -// parseExtraSpecs parses a JSON string of extra specs into a map. -// Returns an empty map if the string is empty or invalid. -func parseExtraSpecs(extraSpecsJSON string) map[string]string { - if extraSpecsJSON == "" { - return make(map[string]string) +// ListVMsByProject returns all VMs for a specific project, querying only that project's servers. +func (s *DBVMSource) ListVMsByProject(ctx context.Context, projectID string) ([]VM, error) { + servers, err := s.NovaReader.GetServersByProject(ctx, projectID) + if err != nil { + return nil, fmt.Errorf("failed to get servers for project: %w", err) } - var extraSpecs map[string]string - if err := json.Unmarshal([]byte(extraSpecsJSON), &extraSpecs); err != nil { - // Log error but don't fail - return empty map - log.Error(err, "failed to parse flavor extra specs JSON", - "extraSpecsJSON", truncateString(extraSpecsJSON, 100)) - return make(map[string]string) + + flavors, err := s.NovaReader.GetAllFlavors(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get flavors: %w", err) + } + type flavorData struct { + VCPUs uint64 + RAM uint64 + Disk uint64 + ExtraSpecs string + } + flavorByName := make(map[string]flavorData, len(flavors)) + for _, f := range flavors { + flavorByName[f.Name] = flavorData{VCPUs: f.VCPUs, RAM: f.RAM, Disk: f.Disk, ExtraSpecs: f.ExtraSpecs} } - return extraSpecs -} -// truncateString truncates a string to maxLen characters, adding "..." if truncated. -func truncateString(s string, maxLen int) string { - if len(s) <= maxLen { - return s + vms := make([]VM, 0, len(servers)) + for _, server := range servers { + if server.OSEXTSRVATTRHost == "" { + continue + } + flavor, ok := flavorByName[server.FlavorName] + if !ok { + continue + } + resources := map[string]resource.Quantity{ + "vcpus": *resource.NewQuantity(int64(flavor.VCPUs), resource.DecimalSI), //nolint:gosec + "memory": *resource.NewQuantity(int64(flavor.RAM)*1024*1024, resource.BinarySI), //nolint:gosec + } + vms = append(vms, VM{ + UUID: server.ID, + Name: server.Name, + Status: server.Status, + FlavorName: server.FlavorName, + ProjectID: server.TenantID, + CurrentHypervisor: server.OSEXTSRVATTRHost, + AvailabilityZone: server.OSEXTAvailabilityZone, + CreatedAt: server.Created, + Resources: resources, + FlavorExtraSpecs: parseExtraSpecs(flavor.ExtraSpecs), + DiskGB: flavor.Disk, + OSType: normalizeOSType(server.OSType), + }) } - return s[:maxLen] + "..." + return vms, nil } // GetVM returns a specific VM by UUID. -// Returns nil, nil if the VM is not found (not an error, just doesn't exist). +// Returns nil, nil if the VM is not found. func (s *DBVMSource) GetVM(ctx context.Context, vmUUID string) (*VM, error) { - // Fetch the server by UUID server, err := s.NovaReader.GetServerByID(ctx, vmUUID) if err != nil { return nil, fmt.Errorf("failed to get server: %w", err) } - if server == nil { - // Server not found + if server == nil || server.OSEXTSRVATTRHost == "" { return nil, nil } - // Skip servers without a host (not yet scheduled) - if server.OSEXTSRVATTRHost == "" { - return nil, nil - } - - // Fetch the flavor for this server flavor, err := s.NovaReader.GetFlavorByName(ctx, server.FlavorName) if err != nil { return nil, fmt.Errorf("failed to get flavor: %w", err) } if flavor == nil { - // Flavor not found return nil, nil } - // Build resources map resources := map[string]resource.Quantity{ - "vcpus": *resource.NewQuantity(int64(flavor.VCPUs), resource.DecimalSI), //nolint:gosec // VCPUs won't overflow int64 - "memory": *resource.NewQuantity(int64(flavor.RAM)*1024*1024, resource.BinarySI), //nolint:gosec // RAM in MB won't overflow int64 + "vcpus": *resource.NewQuantity(int64(flavor.VCPUs), resource.DecimalSI), //nolint:gosec + "memory": *resource.NewQuantity(int64(flavor.RAM)*1024*1024, resource.BinarySI), //nolint:gosec } - // Parse extra specs from JSON string - extraSpecs := parseExtraSpecs(flavor.ExtraSpecs) - return &VM{ UUID: server.ID, + Name: server.Name, + Status: server.Status, FlavorName: server.FlavorName, ProjectID: server.TenantID, CurrentHypervisor: server.OSEXTSRVATTRHost, AvailabilityZone: server.OSEXTAvailabilityZone, CreatedAt: server.Created, Resources: resources, - FlavorExtraSpecs: extraSpecs, + FlavorExtraSpecs: parseExtraSpecs(flavor.ExtraSpecs), + DiskGB: flavor.Disk, + OSType: normalizeOSType(server.OSType), }, nil } // ListVMsOnHypervisors returns VMs that are on the given hypervisors. -// If trustHypervisorLocation is true, uses hypervisor CRD as source of truth for VM location. -// If trustHypervisorLocation is false, uses postgres as source of truth but filters to VMs on known hypervisors. -// Also logs warnings about data sync issues between postgres and hypervisor CRD. func (s *DBVMSource) ListVMsOnHypervisors( ctx context.Context, hypervisorList *hv1.HypervisorList, trustHypervisorLocation bool, ) ([]VM, error) { - // Get VMs from postgres + vms, err := s.ListVMs(ctx) if err != nil { return nil, err } - // Warn about data sync issues warnUnknownVMsOnHypervisors(hypervisorList, vms) if trustHypervisorLocation { - // Use hypervisor CRD as source of truth for VM location result := buildVMsFromHypervisors(hypervisorList, vms) - log.V(1).Info("built VMs from hypervisor instances (TrustHypervisorLocation=true)", + vmSourceLog.V(1).Info("built VMs from hypervisor instances (TrustHypervisorLocation=true)", "count", len(result), "knownHypervisors", len(hypervisorList.Items)) return result, nil } - // Use postgres as source of truth, but filter to VMs on known hypervisors result := filterVMsOnKnownHypervisors(vms, hypervisorList) - log.V(1).Info("filtered VMs to those on known hypervisors and in hypervisor instances", + vmSourceLog.V(1).Info("filtered VMs to those on known hypervisors and in hypervisor instances", "count", len(result), "knownHypervisors", len(hypervisorList.Items)) return result, nil } +// IsServerActive returns true if the server exists in the servers table and is not DELETED. +func (s *DBVMSource) IsServerActive(ctx context.Context, vmUUID string) (bool, error) { + server, err := s.NovaReader.GetServerByID(ctx, vmUUID) + if err != nil { + return false, fmt.Errorf("failed to check server existence: %w", err) + } + if server == nil { + return false, nil + } + return server.Status != "DELETED", nil +} + +// GetDeletedVMInfo returns metadata about a deleted VM from the deleted_servers table. +func (s *DBVMSource) GetDeletedVMInfo(ctx context.Context, vmUUID string) (*DeletedVMInfo, error) { + deletedServer, err := s.NovaReader.GetDeletedServerByID(ctx, vmUUID) + if err != nil { + return nil, fmt.Errorf("failed to get deleted server: %w", err) + } + if deletedServer == nil { + return nil, nil + } + + flavor, err := s.NovaReader.GetFlavorByName(ctx, deletedServer.FlavorName) + if err != nil { + return nil, fmt.Errorf("failed to get flavor for deleted server: %w", err) + } + if flavor == nil { + return nil, fmt.Errorf("flavor %q not found for deleted server %s", deletedServer.FlavorName, vmUUID) + } + + return &DeletedVMInfo{ + ProjectID: deletedServer.TenantID, + AvailabilityZone: deletedServer.OSEXTAvailabilityZone, + FlavorName: deletedServer.FlavorName, + RAMMiB: flavor.RAM, + VCPUs: flavor.VCPUs, + }, nil +} + // ============================================================================ -// VM/Hypervisor Processing (internal helpers) +// Internal helpers // ============================================================================ -// buildVMsFromHypervisors builds VMs from hypervisor instances, using the hypervisor CRD -// as the source of truth for VM location. It enriches VMs with data from postgres (flavor, size, extra specs, AZ). -// This is used when TrustHypervisorLocation is true. -// -// The function: -// 1. Iterates through all hypervisor instances to get VM UUIDs and their actual location -// 2. Looks up each VM in the postgres-sourced vms list to get flavor/size/extra specs/AZ -// 3. Returns VMs that exist in both hypervisor instances AND postgres (need postgres for scheduling data) -// 4. Deduplicates VMs that appear on multiple hypervisors (transient state during live migration) +func parseExtraSpecs(extraSpecsJSON string) map[string]string { + if extraSpecsJSON == "" { + return make(map[string]string) + } + var extraSpecs map[string]string + if err := json.Unmarshal([]byte(extraSpecsJSON), &extraSpecs); err != nil { + vmSourceLog.Error(err, "failed to parse flavor extra specs JSON", + "extraSpecsJSON", truncateString(extraSpecsJSON, 100)) + return make(map[string]string) + } + return extraSpecs +} + +func truncateString(s string, maxLen int) string { + if len(s) <= maxLen { + return s + } + return s[:maxLen] + "..." +} + +// normalizeOSType returns "unknown" for empty OS type strings. +func normalizeOSType(osType string) string { + if osType == "" { + return "unknown" + } + return osType +} + func buildVMsFromHypervisors(hypervisorList *hv1.HypervisorList, postgresVMs []VM) []VM { - // Build a map of VM UUID -> VM data from postgres for quick lookup vmDataByUUID := make(map[string]VM, len(postgresVMs)) for _, vm := range postgresVMs { vmDataByUUID[vm.UUID] = vm @@ -291,21 +363,15 @@ func buildVMsFromHypervisors(hypervisorList *hv1.HypervisorList, postgresVMs []V var result []VM var enrichedCount, notInPostgresCount, duplicateCount int + seen := make(map[string]string) - // Track seen UUIDs to deduplicate VMs that appear on multiple hypervisors - // This can happen transiently during live migration - seen := make(map[string]string) // vmUUID -> first hypervisor seen - - // Iterate through hypervisor instances for _, hv := range hypervisorList.Items { for _, inst := range hv.Status.Instances { if !inst.Active { continue } - - // Check for duplicate UUIDs (same VM on multiple hypervisors) if firstHypervisor, alreadySeen := seen[inst.ID]; alreadySeen { - log.Info("duplicate VM UUID on multiple hypervisors, skipping", + vmSourceLog.Info("duplicate VM UUID on multiple hypervisors, skipping", "uuid", inst.ID, "hypervisor", hv.Name, "firstSeenOn", firstHypervisor) @@ -314,31 +380,32 @@ func buildVMsFromHypervisors(hypervisorList *hv1.HypervisorList, postgresVMs []V } seen[inst.ID] = hv.Name - // Look up VM data from postgres pgVM, existsInPostgres := vmDataByUUID[inst.ID] if !existsInPostgres { - // VM is on hypervisor but not in postgres - skip (need postgres for flavor/size) notInPostgresCount++ continue } - // Build VM with hypervisor location but postgres data (including AZ) vm := VM{ UUID: inst.ID, + Name: pgVM.Name, + Status: pgVM.Status, FlavorName: pgVM.FlavorName, ProjectID: pgVM.ProjectID, - CurrentHypervisor: hv.Name, // Use hypervisor CRD location, not postgres + CurrentHypervisor: hv.Name, AvailabilityZone: pgVM.AvailabilityZone, CreatedAt: pgVM.CreatedAt, Resources: pgVM.Resources, FlavorExtraSpecs: pgVM.FlavorExtraSpecs, + DiskGB: pgVM.DiskGB, + OSType: pgVM.OSType, } result = append(result, vm) enrichedCount++ } } - log.V(1).Info("buildVMsFromHypervisors statistics", + vmSourceLog.V(1).Info("buildVMsFromHypervisors statistics", "totalHypervisorInstances", enrichedCount+notInPostgresCount+duplicateCount, "enrichedWithPostgresData", enrichedCount, "notInPostgres", notInPostgresCount, @@ -347,23 +414,14 @@ func buildVMsFromHypervisors(hypervisorList *hv1.HypervisorList, postgresVMs []V return result } -// filterVMsOnKnownHypervisors filters VMs to only include those that: -// 1. Are running on a known hypervisor -// 2. Are actually listed in that hypervisor's Status.Instances -// This removes VMs that are on hypervisors not managed by the hypervisor operator, -// or VMs that claim to be on a hypervisor but aren't in its instances list (data sync issue). func filterVMsOnKnownHypervisors(vms []VM, hypervisorList *hv1.HypervisorList) []VM { - // Build a set of known hypervisors for O(1) lookup hypervisorSet := make(map[string]bool, len(hypervisorList.Items)) for _, hv := range hypervisorList.Items { hypervisorSet[hv.Name] = true } - // Build a set of VM UUIDs that are actually in hypervisor instances - // Key: "vmUUID:hypervisorName" to ensure VM is on the correct hypervisor vmOnHypervisor := make(map[string]bool) - // Also track all VMs on hypervisors (regardless of which hypervisor) - allVMsOnHypervisors := make(map[string]string) // vmUUID -> hypervisorName + allVMsOnHypervisors := make(map[string]string) totalVMsOnHypervisors := 0 for _, hv := range hypervisorList.Items { for _, inst := range hv.Status.Instances { @@ -377,21 +435,16 @@ func filterVMsOnKnownHypervisors(vms []VM, hypervisorList *hv1.HypervisorList) [ } var result []VM - var filteredUnknownHypervisor int - var filteredNotInInstances int - var filteredWrongHypervisor int + var filteredUnknownHypervisor, filteredNotInInstances, filteredWrongHypervisor int for _, vm := range vms { - // Check if hypervisor is known if !hypervisorSet[vm.CurrentHypervisor] { filteredUnknownHypervisor++ continue } - // Check if VM is actually in the hypervisor's instances list key := vm.UUID + ":" + vm.CurrentHypervisor if !vmOnHypervisor[key] { - // Check if VM is on a different hypervisor if actualHypervisor, exists := allVMsOnHypervisors[vm.UUID]; exists { - log.V(2).Info("VM claims to be on one hypervisor but is actually on another", + vmSourceLog.V(2).Info("VM claims to be on one hypervisor but is actually on another", "vmUUID", vm.UUID, "claimedHypervisor", vm.CurrentHypervisor, "actualHypervisor", actualHypervisor) @@ -406,7 +459,7 @@ func filterVMsOnKnownHypervisors(vms []VM, hypervisorList *hv1.HypervisorList) [ totalFiltered := filteredUnknownHypervisor + filteredNotInInstances + filteredWrongHypervisor if totalFiltered > 0 { - log.Info("filterVMsOnKnownHypervisors statistics", + vmSourceLog.Info("filterVMsOnKnownHypervisors statistics", "inputVMs", len(vms), "totalVMsOnHypervisors", totalVMsOnHypervisors, "filteredUnknownHypervisor", filteredUnknownHypervisor, @@ -419,60 +472,12 @@ func filterVMsOnKnownHypervisors(vms []VM, hypervisorList *hv1.HypervisorList) [ return result } -// IsServerActive returns true if the server exists in the servers table and is not DELETED. -// VMs in any other status (ACTIVE, SHUTOFF, MIGRATING, ERROR, etc.) still consume resources -// and should NOT be decremented from quota usage. -// Used by the quota controller to distinguish deleted VMs from migrated/existing ones. -func (s *DBVMSource) IsServerActive(ctx context.Context, vmUUID string) (bool, error) { - server, err := s.NovaReader.GetServerByID(ctx, vmUUID) - if err != nil { - return false, fmt.Errorf("failed to check server existence: %w", err) - } - if server == nil { - return false, nil - } - return server.Status != "DELETED", nil -} - -// GetDeletedVMInfo returns metadata about a deleted VM from the deleted_servers table, -// including resolved flavor resources. Returns nil, nil if the VM is not found in deleted_servers. -func (s *DBVMSource) GetDeletedVMInfo(ctx context.Context, vmUUID string) (*DeletedVMInfo, error) { - deletedServer, err := s.NovaReader.GetDeletedServerByID(ctx, vmUUID) - if err != nil { - return nil, fmt.Errorf("failed to get deleted server: %w", err) - } - if deletedServer == nil { - return nil, nil - } - - // Resolve the flavor to get RAM/VCPUs - flavor, err := s.NovaReader.GetFlavorByName(ctx, deletedServer.FlavorName) - if err != nil { - return nil, fmt.Errorf("failed to get flavor for deleted server: %w", err) - } - if flavor == nil { - return nil, fmt.Errorf("flavor %q not found for deleted server %s", deletedServer.FlavorName, vmUUID) - } - - return &DeletedVMInfo{ - ProjectID: deletedServer.TenantID, - AvailabilityZone: deletedServer.OSEXTAvailabilityZone, - FlavorName: deletedServer.FlavorName, - RAMMiB: flavor.RAM, - VCPUs: flavor.VCPUs, - }, nil -} - -// warnUnknownVMsOnHypervisors logs a warning for VMs that are on hypervisors but not in the ListVMs (i.e. nova) result. -// This can indicate a data sync issue between the hypervisor operator and the VM datasource. func warnUnknownVMsOnHypervisors(hypervisors *hv1.HypervisorList, vms []VM) { - // Build a set of VM UUIDs from ListVMs vmUUIDs := make(map[string]bool, len(vms)) for _, vm := range vms { vmUUIDs[vm.UUID] = true } - // Build a set of VM UUIDs from hypervisors hypervisorVMUUIDs := make(map[string]bool) for _, hv := range hypervisors.Items { for _, inst := range hv.Status.Instances { @@ -482,12 +487,11 @@ func warnUnknownVMsOnHypervisors(hypervisors *hv1.HypervisorList, vms []VM) { } } - // Check each hypervisor's instances - VMs on hypervisors but not in ListVMs vmsOnHypervisorsNotInListVMs := 0 for _, hv := range hypervisors.Items { for _, inst := range hv.Status.Instances { if inst.Active && !vmUUIDs[inst.ID] { - log.Info("WARNING: VM on hypervisor not found in ListVMs - possible data sync issue", + vmSourceLog.Info("WARNING: VM on hypervisor not found in ListVMs - possible data sync issue", "vmUUID", inst.ID, "vmName", inst.Name, "hypervisor", hv.Name) @@ -496,7 +500,6 @@ func warnUnknownVMsOnHypervisors(hypervisors *hv1.HypervisorList, vms []VM) { } } - // Check VMs in ListVMs but not on any hypervisor vmsInListVMsNotOnHypervisors := 0 for _, vm := range vms { if !hypervisorVMUUIDs[vm.UUID] { @@ -505,14 +508,92 @@ func warnUnknownVMsOnHypervisors(hypervisors *hv1.HypervisorList, vms []VM) { } if vmsOnHypervisorsNotInListVMs > 0 { - log.V(1).Info("VMs on hypervisors not found in ListVMs", + vmSourceLog.V(1).Info("VMs on hypervisors not found in ListVMs", "count", vmsOnHypervisorsNotInListVMs, "hint", "This may indicate a data sync issue between hypervisor operator and nova servers") } - if vmsInListVMsNotOnHypervisors > 0 { - log.V(1).Info("VMs in ListVMs not found on any hypervisor", + vmSourceLog.V(1).Info("VMs in ListVMs not found on any hypervisor", "count", vmsInListVMsNotOnHypervisors, "hint", "This may indicate a data sync issue between nova servers and hypervisor operator") } } + +// lazyDBVMSource is a VMSource that defers Postgres connection until first use. +// Postgres connection requires the manager cache to be ready (Datasource CRD lookup), +// so this type is safe to construct during setup before the manager starts. +type lazyDBVMSource struct { + k8sClient client.Client + datasourceName string + mu sync.Mutex + inner *DBVMSource +} + +// NewPostgresVMSource creates a VMSource backed by the named Datasource CRD. +// The Postgres connection is established on first use, so this is safe to call +// before the manager cache is ready. +func NewPostgresVMSource(k8sClient client.Client, datasourceName string) VMSource { + return &lazyDBVMSource{k8sClient: k8sClient, datasourceName: datasourceName} +} + +func (s *lazyDBVMSource) get(ctx context.Context) (*DBVMSource, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.inner != nil { + return s.inner, nil + } + postgresReader, err := external.NewPostgresReader(ctx, s.k8sClient, s.datasourceName) + if err != nil { + return nil, fmt.Errorf("failed to connect to datasource %s: %w", s.datasourceName, err) + } + s.inner = NewDBVMSource(external.NewNovaReader(postgresReader)) + return s.inner, nil +} + +func (s *lazyDBVMSource) ListVMs(ctx context.Context) ([]VM, error) { + inner, err := s.get(ctx) + if err != nil { + return nil, err + } + return inner.ListVMs(ctx) +} + +func (s *lazyDBVMSource) ListVMsByProject(ctx context.Context, projectID string) ([]VM, error) { + inner, err := s.get(ctx) + if err != nil { + return nil, err + } + return inner.ListVMsByProject(ctx, projectID) +} + +func (s *lazyDBVMSource) ListVMsOnHypervisors(ctx context.Context, hypervisorList *hv1.HypervisorList, trustHypervisorLocation bool) ([]VM, error) { + inner, err := s.get(ctx) + if err != nil { + return nil, err + } + return inner.ListVMsOnHypervisors(ctx, hypervisorList, trustHypervisorLocation) +} + +func (s *lazyDBVMSource) GetVM(ctx context.Context, vmUUID string) (*VM, error) { + inner, err := s.get(ctx) + if err != nil { + return nil, err + } + return inner.GetVM(ctx, vmUUID) +} + +func (s *lazyDBVMSource) IsServerActive(ctx context.Context, vmUUID string) (bool, error) { + inner, err := s.get(ctx) + if err != nil { + return false, err + } + return inner.IsServerActive(ctx, vmUUID) +} + +func (s *lazyDBVMSource) GetDeletedVMInfo(ctx context.Context, vmUUID string) (*DeletedVMInfo, error) { + inner, err := s.get(ctx) + if err != nil { + return nil, err + } + return inner.GetDeletedVMInfo(ctx, vmUUID) +} diff --git a/internal/scheduling/reservations/failover/vm_source_test.go b/internal/scheduling/reservations/vm_source_test.go similarity index 80% rename from internal/scheduling/reservations/failover/vm_source_test.go rename to internal/scheduling/reservations/vm_source_test.go index a710c5658..74a8d517b 100644 --- a/internal/scheduling/reservations/failover/vm_source_test.go +++ b/internal/scheduling/reservations/vm_source_test.go @@ -1,7 +1,7 @@ // Copyright SAP SE // SPDX-License-Identifier: Apache-2.0 -package failover +package reservations import ( "context" @@ -156,7 +156,7 @@ func TestDBVMSource_GetVM(t *testing.T) { vmID: "non-existent-vm", mock: &mockNovaReader{ getServerByIDFunc: func(ctx context.Context, serverID string) (*nova.Server, error) { - return nil, nil // Server not found + return nil, nil }, }, wantErr: false, @@ -320,30 +320,75 @@ func TestBuildVMsFromHypervisors(t *testing.T) { } } -func TestFilterVMsOnKnownHypervisors_NilInputs(t *testing.T) { +func TestFilterVMsOnKnownHypervisors(t *testing.T) { tests := []struct { name string vms []VM hypervisorList *hv1.HypervisorList - wantCount int + expectedCount int + expectedUUIDs []string }{ { - name: "nil hypervisor items does not panic", - vms: []VM{{UUID: "vm-1", CurrentHypervisor: "host1"}}, + name: "empty VMs list", + vms: []VM{}, hypervisorList: &hv1.HypervisorList{ - Items: nil, + Items: []hv1.Hypervisor{ + {ObjectMeta: metav1.ObjectMeta{Name: "host1"}}, + }, }, - wantCount: 0, + expectedCount: 0, }, { - name: "nil VMs does not panic", - vms: nil, + name: "all VMs on known hypervisors and in instances", + vms: []VM{ + {UUID: "vm-1", CurrentHypervisor: "host1"}, + {UUID: "vm-2", CurrentHypervisor: "host2"}, + }, hypervisorList: &hv1.HypervisorList{ Items: []hv1.Hypervisor{ - {ObjectMeta: metav1.ObjectMeta{Name: "host1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "host1"}, Status: hv1.HypervisorStatus{Instances: []hv1.Instance{{ID: "vm-1", Active: true}}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "host2"}, Status: hv1.HypervisorStatus{Instances: []hv1.Instance{{ID: "vm-2", Active: true}}}}, + }, + }, + expectedCount: 2, + expectedUUIDs: []string{"vm-1", "vm-2"}, + }, + { + name: "VM claims hypervisor but not in instances list", + vms: []VM{ + {UUID: "vm-1", CurrentHypervisor: "host1"}, + {UUID: "vm-2", CurrentHypervisor: "host2"}, + }, + hypervisorList: &hv1.HypervisorList{ + Items: []hv1.Hypervisor{ + {ObjectMeta: metav1.ObjectMeta{Name: "host1"}, Status: hv1.HypervisorStatus{Instances: []hv1.Instance{{ID: "vm-1", Active: true}}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "host2"}}, + }, + }, + expectedCount: 1, + expectedUUIDs: []string{"vm-1"}, + }, + { + name: "inactive VM in instances", + vms: []VM{{UUID: "vm-1", CurrentHypervisor: "host1"}}, + hypervisorList: &hv1.HypervisorList{ + Items: []hv1.Hypervisor{ + {ObjectMeta: metav1.ObjectMeta{Name: "host1"}, Status: hv1.HypervisorStatus{Instances: []hv1.Instance{{ID: "vm-1", Active: false}}}}, }, }, - wantCount: 0, + expectedCount: 0, + }, + { + name: "nil hypervisor items does not panic", + vms: []VM{{UUID: "vm-1", CurrentHypervisor: "host1"}}, + hypervisorList: &hv1.HypervisorList{Items: nil}, + expectedCount: 0, + }, + { + name: "nil VMs does not panic", + vms: nil, + hypervisorList: &hv1.HypervisorList{Items: []hv1.Hypervisor{{ObjectMeta: metav1.ObjectMeta{Name: "host1"}}}}, + expectedCount: 0, }, } @@ -354,17 +399,24 @@ func TestFilterVMsOnKnownHypervisors_NilInputs(t *testing.T) { t.Errorf("filterVMsOnKnownHypervisors panicked: %v", r) } }() - result := filterVMsOnKnownHypervisors(tt.vms, tt.hypervisorList) - - if len(result) != tt.wantCount { - t.Errorf("expected %d VMs, got %d", tt.wantCount, len(result)) + if len(result) != tt.expectedCount { + t.Errorf("expected %d VMs, got %d", tt.expectedCount, len(result)) + } + resultUUIDs := make(map[string]bool) + for _, vm := range result { + resultUUIDs[vm.UUID] = true + } + for _, uuid := range tt.expectedUUIDs { + if !resultUUIDs[uuid] { + t.Errorf("expected VM %s in result, but not found", uuid) + } } }) } } -// mockNovaReader implements NovaReader for testing. +// mockNovaReader implements NovaReaderInterface for testing. type mockNovaReader struct { getAllServersFunc func(ctx context.Context) ([]nova.Server, error) getAllFlavorsFunc func(ctx context.Context) ([]nova.Flavor, error) @@ -403,3 +455,7 @@ func (m *mockNovaReader) GetFlavorByName(ctx context.Context, flavorName string) func (m *mockNovaReader) GetDeletedServerByID(_ context.Context, _ string) (*nova.DeletedServer, error) { return nil, nil } + +func (m *mockNovaReader) GetServersByProject(_ context.Context, _ string) ([]nova.Server, error) { + return nil, nil +}