From 64e58d05b9e4c760568bc35d1f8b43803a87f57d Mon Sep 17 00:00:00 2001 From: Markus Wieland Date: Thu, 2 Jul 2026 10:17:38 +0200 Subject: [PATCH] refactor(nova): remove image datasource --- api/v1alpha1/datasource_types.go | 1 - .../cortex-nova/templates/datasources.yaml | 24 ------ .../plugins/openstack/controller_test.go | 1 - .../plugins/openstack/nova/nova_api.go | 86 ------------------- .../plugins/openstack/nova/nova_api_test.go | 60 ------------- .../plugins/openstack/nova/nova_sync.go | 24 ------ .../plugins/openstack/nova/nova_sync_test.go | 36 -------- .../plugins/openstack/nova/nova_types.go | 14 --- 8 files changed, 246 deletions(-) diff --git a/api/v1alpha1/datasource_types.go b/api/v1alpha1/datasource_types.go index f9963a35c..fff321c48 100644 --- a/api/v1alpha1/datasource_types.go +++ b/api/v1alpha1/datasource_types.go @@ -52,7 +52,6 @@ const ( NovaDatasourceTypeFlavors NovaDatasourceType = "flavors" NovaDatasourceTypeMigrations NovaDatasourceType = "migrations" NovaDatasourceTypeAggregates NovaDatasourceType = "aggregates" - NovaDatasourceTypeImages NovaDatasourceType = "images" ) type NovaDatasource struct { diff --git a/helm/bundles/cortex-nova/templates/datasources.yaml b/helm/bundles/cortex-nova/templates/datasources.yaml index 582effac2..f9160602f 100644 --- a/helm/bundles/cortex-nova/templates/datasources.yaml +++ b/helm/bundles/cortex-nova/templates/datasources.yaml @@ -337,30 +337,6 @@ spec: --- apiVersion: cortex.cloud/v1alpha1 kind: Datasource -metadata: - name: nova-images -spec: - schedulingDomain: nova - databaseSecretRef: - name: cortex-nova-postgres - namespace: {{ .Release.Namespace }} - {{- if .Values.openstack.sso.enabled }} - ssoSecretRef: - name: cortex-nova-openstack-sso - namespace: {{ .Release.Namespace }} - {{- end }} - type: openstack - openstack: - syncInterval: 3600s - secretRef: - name: cortex-nova-openstack-keystone - namespace: {{ .Release.Namespace }} - type: nova - nova: - type: images ---- -apiVersion: cortex.cloud/v1alpha1 -kind: Datasource metadata: name: limes-project-commitments spec: diff --git a/internal/knowledge/datasources/plugins/openstack/controller_test.go b/internal/knowledge/datasources/plugins/openstack/controller_test.go index 586238d54..899e83237 100644 --- a/internal/knowledge/datasources/plugins/openstack/controller_test.go +++ b/internal/knowledge/datasources/plugins/openstack/controller_test.go @@ -104,7 +104,6 @@ func TestNovaDatasourceTypeConstants(t *testing.T) { {v1alpha1.NovaDatasourceTypeFlavors, "flavors"}, {v1alpha1.NovaDatasourceTypeMigrations, "migrations"}, {v1alpha1.NovaDatasourceTypeAggregates, "aggregates"}, - {v1alpha1.NovaDatasourceTypeImages, "images"}, } for _, test := range tests { diff --git a/internal/knowledge/datasources/plugins/openstack/nova/nova_api.go b/internal/knowledge/datasources/plugins/openstack/nova/nova_api.go index 906a46a1e..b7e25c2ba 100644 --- a/internal/knowledge/datasources/plugins/openstack/nova/nova_api.go +++ b/internal/knowledge/datasources/plugins/openstack/nova/nova_api.go @@ -10,18 +10,15 @@ import ( "log/slog" "net/http" "net/url" - "strings" "time" "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/datasources" "github.com/cobaltcore-dev/cortex/pkg/keystone" "github.com/gophercloud/gophercloud/v2" - "github.com/gophercloud/gophercloud/v2/openstack" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/aggregates" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/flavors" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" - glanceimages "github.com/gophercloud/gophercloud/v2/openstack/image/v2/images" "github.com/gophercloud/gophercloud/v2/pagination" "github.com/prometheus/client_golang/prometheus" "github.com/sapcc/go-bits/liquidapi" @@ -43,8 +40,6 @@ type NovaAPI interface { GetAllMigrations(ctx context.Context) ([]Migration, error) // Get all aggregates. GetAllAggregates(ctx context.Context) ([]Aggregate, error) - // Get all Glance images with pre-computed os_type. - GetAllImages(ctx context.Context) ([]Image, error) } // API for OpenStack Nova. @@ -57,8 +52,6 @@ type novaAPI struct { conf v1alpha1.NovaDatasource // Authenticated OpenStack compute service client. sc *gophercloud.ServiceClient - // Authenticated Glance image service client (only used for NovaDatasourceTypeImages). - glance *gophercloud.ServiceClient // OS type prober for determining VM operating system type (only for NovaDatasourceTypeServers). osTypeProber *liquidapi.OSTypeProber } @@ -90,16 +83,6 @@ func (api *novaAPI) Init(ctx context.Context) error { // Since 2.61, the extra_specs are returned in the flavor details. Microversion: "2.61", } - // Initialize the Glance client only when this datasource is used for images. - if api.conf.Type == v1alpha1.NovaDatasourceTypeImages { - glanceClient, err := openstack.NewImageV2(provider, gophercloud.EndpointOpts{ - Availability: gophercloud.Availability(sameAsKeystone), - }) - if err != nil { - return fmt.Errorf("failed to create Glance client: %w", err) - } - api.glance = glanceClient - } // Initialize the OS type prober only for the servers datasource. if api.conf.Type == v1alpha1.NovaDatasourceTypeServers { eo := gophercloud.EndpointOpts{Availability: gophercloud.Availability(sameAsKeystone)} @@ -510,75 +493,6 @@ func (api *novaAPI) GetAllAggregates(ctx context.Context) ([]Aggregate, error) { return aggregates, nil } -// GetAllImages fetches all Glance images and returns them with pre-computed os_type. -// See deriveOSType for the derivation logic. -func (api *novaAPI) GetAllImages(ctx context.Context) ([]Image, error) { - if api.glance == nil { - return nil, fmt.Errorf("glance client not initialized: datasource type must be %q", v1alpha1.NovaDatasourceTypeImages) - } - - label := Image{}.TableName() - slog.Info("fetching nova data", "label", label) - if api.mon.RequestTimer != nil { - hist := api.mon.RequestTimer.WithLabelValues(label) - timer := prometheus.NewTimer(hist) - defer timer.ObserveDuration() - } - - var result []Image - opts := glanceimages.ListOpts{Limit: 1000} - err := glanceimages.List(api.glance, opts).EachPage(ctx, func(_ context.Context, page pagination.Page) (bool, error) { - imgs, err := glanceimages.ExtractImages(page) - if err != nil { - return false, err - } - for _, img := range imgs { - result = append(result, Image{ - ID: img.ID, - OSType: deriveOSType(img.Properties, img.Tags), - }) - } - return true, nil - }) - if err != nil { - return nil, fmt.Errorf("failed to list Glance images: %w", err) - } - slog.Info("fetched", "label", label, "count", len(result)) - return result, nil -} - -// deriveOSType computes os_type from image properties and tags. -// Mirrors the logic of OSTypeProber.findFromImage in github.com/sapcc/go-bits/liquidapi, -// with two intentional simplifications: -// 1. No regex validation on vmware_ostype — Nova validates that field at VM boot time, -// so any value stored in Glance is already valid. -// 2. Volume-booted VMs are not yet supported — os_type will be "unknown" for them. -// Supporting them would require per-VM Cinder calls (volume_image_metadata.vmware_ostype) -// either at server sync time or via a dedicated datasource. -func deriveOSType(properties map[string]any, tags []string) string { - if v, ok := properties["vmware_ostype"]; ok { - if s, ok := v.(string); ok && s != "" { - return s - } - } - var osType string - for _, tag := range tags { - if after, ok := strings.CutPrefix(tag, "ostype:"); ok { - if osType == "" { - osType = after - } else { - // multiple ostype: tags → ambiguous, fall through to unknown - osType = "" - break - } - } - } - if osType != "" { - return osType - } - return "unknown" -} - // initOSTypeProber safely creates an OSTypeProber, returning nil on any error or panic. func initOSTypeProber(provider *gophercloud.ProviderClient, eo gophercloud.EndpointOpts) (prober *liquidapi.OSTypeProber) { defer func() { diff --git a/internal/knowledge/datasources/plugins/openstack/nova/nova_api_test.go b/internal/knowledge/datasources/plugins/openstack/nova/nova_api_test.go index 63f83c176..49f0af4b4 100644 --- a/internal/knowledge/datasources/plugins/openstack/nova/nova_api_test.go +++ b/internal/knowledge/datasources/plugins/openstack/nova/nova_api_test.go @@ -538,66 +538,6 @@ func TestNovaAPI_GetAllHypervisors_DeduplicatesHypervisors(t *testing.T) { } } -func TestDeriveOSType(t *testing.T) { - tests := []struct { - name string - properties map[string]any - tags []string - want string - }{ - { - name: "vmware_ostype property wins", - properties: map[string]any{"vmware_ostype": "windows8Server64Guest"}, - tags: []string{"ostype:linux"}, - want: "windows8Server64Guest", - }, - { - name: "vmware_ostype empty string falls through to tags", - properties: map[string]any{"vmware_ostype": ""}, - tags: []string{"ostype:debian"}, - want: "debian", - }, - { - name: "vmware_ostype not a string falls through", - properties: map[string]any{"vmware_ostype": 42}, - tags: []string{"ostype:centos"}, - want: "centos", - }, - { - name: "single ostype tag", - properties: map[string]any{}, - tags: []string{"ostype:ubuntu", "env:prod"}, - want: "ubuntu", - }, - { - name: "multiple ostype tags: ambiguous, returns unknown", - properties: map[string]any{}, - tags: []string{"ostype:ubuntu", "ostype:debian"}, - want: "unknown", - }, - { - name: "no properties, no tags", - properties: map[string]any{}, - tags: nil, - want: "unknown", - }, - { - name: "tags without ostype prefix", - properties: map[string]any{}, - tags: []string{"env:prod", "region:eu"}, - want: "unknown", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := deriveOSType(tt.properties, tt.tags); got != tt.want { - t.Errorf("deriveOSType() = %q, want %q", got, tt.want) - } - }) - } -} - func TestNovaAPI_GetAllMigrations_DeduplicatesMigrations(t *testing.T) { tests := []struct { name string diff --git a/internal/knowledge/datasources/plugins/openstack/nova/nova_sync.go b/internal/knowledge/datasources/plugins/openstack/nova/nova_sync.go index ec0533f84..a47449c27 100644 --- a/internal/knowledge/datasources/plugins/openstack/nova/nova_sync.go +++ b/internal/knowledge/datasources/plugins/openstack/nova/nova_sync.go @@ -46,8 +46,6 @@ func (s *NovaSyncer) Init(ctx context.Context) error { tables = append(tables, s.DB.AddTable(Migration{})) case v1alpha1.NovaDatasourceTypeAggregates: tables = append(tables, s.DB.AddTable(Aggregate{})) - case v1alpha1.NovaDatasourceTypeImages: - tables = append(tables, s.DB.AddTable(Image{})) } return s.DB.CreateTable(tables...) } @@ -70,8 +68,6 @@ func (s *NovaSyncer) Sync(ctx context.Context) (int64, error) { nResults, err = s.SyncAllMigrations(ctx) case v1alpha1.NovaDatasourceTypeAggregates: nResults, err = s.SyncAllAggregates(ctx) - case v1alpha1.NovaDatasourceTypeImages: - nResults, err = s.SyncAllImages(ctx) } return nResults, err } @@ -202,26 +198,6 @@ func (s *NovaSyncer) SyncAllMigrations(ctx context.Context) (int64, error) { return int64(len(allMigrations)), nil } -// Sync all Glance images into the database with pre-computed os_type. -func (s *NovaSyncer) SyncAllImages(ctx context.Context) (int64, error) { - allImages, err := s.API.GetAllImages(ctx) - if err != nil { - return 0, err - } - err = db.ReplaceAll(s.DB, allImages...) - if err != nil { - return 0, err - } - label := Image{}.TableName() - if s.Mon.ObjectsGauge != nil { - s.Mon.ObjectsGauge.WithLabelValues(label).Set(float64(len(allImages))) - } - if s.Mon.RequestProcessedCounter != nil { - s.Mon.RequestProcessedCounter.WithLabelValues(label).Inc() - } - return int64(len(allImages)), nil -} - // Sync the OpenStack aggregates into the database. func (s *NovaSyncer) SyncAllAggregates(ctx context.Context) (int64, error) { allAggregates, err := s.API.GetAllAggregates(ctx) diff --git a/internal/knowledge/datasources/plugins/openstack/nova/nova_sync_test.go b/internal/knowledge/datasources/plugins/openstack/nova/nova_sync_test.go index fdd826171..9761b9a93 100644 --- a/internal/knowledge/datasources/plugins/openstack/nova/nova_sync_test.go +++ b/internal/knowledge/datasources/plugins/openstack/nova/nova_sync_test.go @@ -55,10 +55,6 @@ func (m *mockNovaAPI) GetAllAggregates(ctx context.Context) ([]Aggregate, error) return []Aggregate{{Name: "aggregate1"}}, nil } -func (m *mockNovaAPI) GetAllImages(ctx context.Context) ([]Image, error) { - return []Image{{ID: "img-1", OSType: "windows8Server64Guest"}}, nil -} - func TestNovaSyncer_Init(t *testing.T) { dbEnv := testlibDB.SetupDBEnv(t) testDB := db.DB{DbMap: dbEnv.DbMap} @@ -271,35 +267,3 @@ func TestNovaSyncer_SyncAggregates(t *testing.T) { t.Fatalf("expected 1 aggregate, got %d", n) } } - -func TestNovaSyncer_SyncImages(t *testing.T) { - dbEnv := testlibDB.SetupDBEnv(t) - testDB := db.DB{DbMap: dbEnv.DbMap} - defer dbEnv.Close() - mon := datasources.Monitor{} - syncer := &NovaSyncer{ - DB: testDB, - Mon: mon, - Conf: v1alpha1.NovaDatasource{Type: v1alpha1.NovaDatasourceTypeImages}, - API: &mockNovaAPI{}, - } - - ctx := t.Context() - if err := syncer.Init(ctx); err != nil { - t.Fatalf("failed to init images syncer: %v", err) - } - n, err := syncer.Sync(ctx) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if n != 1 { - t.Fatalf("expected 1 image, got %d", n) - } - var images []Image - if _, err := testDB.Select(&images, "SELECT * FROM "+Image{}.TableName()); err != nil { - t.Fatalf("select images: %v", err) - } - if len(images) != 1 || images[0].ID != "img-1" || images[0].OSType != "windows8Server64Guest" { - t.Errorf("unexpected images in DB: %+v", images) - } -} diff --git a/internal/knowledge/datasources/plugins/openstack/nova/nova_types.go b/internal/knowledge/datasources/plugins/openstack/nova/nova_types.go index 17c422194..544b74572 100644 --- a/internal/knowledge/datasources/plugins/openstack/nova/nova_types.go +++ b/internal/knowledge/datasources/plugins/openstack/nova/nova_types.go @@ -512,17 +512,3 @@ func (Aggregate) TableName() string { return "openstack_aggregates_v2" } // Index for the openstack model. func (Aggregate) Indexes() map[string][]string { return nil } - -// Image stores pre-computed os_type for a Glance image UUID. -// Populated by the NovaDatasourceTypeImages syncer from the Glance API. -// Used by the CR usage API to include os_type in VM subresources without live API calls. -type Image struct { - ID string `json:"id" db:"id,primarykey"` - OSType string `json:"os_type" db:"os_type"` -} - -// Table in which the openstack model is stored. -func (Image) TableName() string { return "openstack_images" } - -// Index for the openstack model. -func (Image) Indexes() map[string][]string { return nil }