Add error handling to ReplaceAll

internal/knowledge/datasources/plugins/openstack/cinder/cinder_sync_test.go

@@ -55,6 +55,9 @@ func TestCinderSyncer_Sync(t *testing.T) {
 	}
 
 	ctx := t.Context()
+	if err := syncer.Init(ctx); err != nil {
+		t.Fatalf("failed to init cinder syncer: %v", err)
+	}
 	_, err := syncer.Sync(ctx)
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
@@ -76,6 +79,9 @@ func TestCinderSyncer_SyncAllStoragePools(t *testing.T) {
 	}
 
 	ctx := t.Context()
+	if err := syncer.Init(ctx); err != nil {
+		t.Fatalf("failed to init cinder syncer: %v", err)
+	}
 	n, err := syncer.SyncAllStoragePools(ctx)
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)

internal/knowledge/datasources/plugins/openstack/limes/limes_sync_test.go

@@ -125,16 +125,18 @@ func TestLimesSyncer_SyncCommitments(t *testing.T) {
 	}
 
 	mon := datasources.Monitor{}
-	k := &testlibKeystone.MockKeystoneClient{}
 	conf := v1alpha1.LimesDatasource{Type: v1alpha1.LimesDatasourceTypeProjectCommitments}
 
 	syncer := &LimesSyncer{
 		DB:   testDB,
 		Mon:  mon,
 		Conf: conf,
-		API:  NewLimesAPI(mon, k, conf),
+		API:  &mockLimesAPI{},
+	}
+
+	if err := syncer.Init(t.Context()); err != nil {
+		t.Fatalf("failed to init limes syncer: %v", err)
 	}
-	syncer.API = &mockLimesAPI{}
 
 	ctx := t.Context()
 	n, err := syncer.SyncCommitments(ctx)

internal/knowledge/datasources/plugins/openstack/manila/manila_sync_test.go

@@ -55,6 +55,9 @@ func TestManilaSyncer_Sync(t *testing.T) {
 	}
 
 	ctx := t.Context()
+	if err := syncer.Init(ctx); err != nil {
+		t.Fatalf("failed to init manila syncer: %v", err)
+	}
 	_, err := syncer.Sync(ctx)
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
@@ -76,6 +79,9 @@ func TestManilaSyncer_SyncAllStoragePools(t *testing.T) {
 	}
 
 	ctx := t.Context()
+	if err := syncer.Init(ctx); err != nil {
+		t.Fatalf("failed to init manila syncer: %v", err)
+	}
 	n, err := syncer.SyncAllStoragePools(ctx)
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)

internal/knowledge/datasources/plugins/openstack/placement/placement_sync_test.go

@@ -55,18 +55,19 @@ func TestPlacementSyncer_Sync(t *testing.T) {
 	testDB := db.DB{DbMap: dbEnv.DbMap}
 	defer dbEnv.Close()
 	mon := datasources.Monitor{}
-	pc := &testlibKeystone.MockKeystoneClient{}
 	conf := v1alpha1.PlacementDatasource{Type: v1alpha1.PlacementDatasourceTypeResourceProviders}
 
 	syncer := &PlacementSyncer{
 		DB:   testDB,
 		Mon:  mon,
 		Conf: conf,
-		API:  NewPlacementAPI(mon, pc, conf),
+		API:  &mockPlacementAPI{},
 	}
-	syncer.API = &mockPlacementAPI{}
 
 	ctx := t.Context()
+	if err := syncer.Init(ctx); err != nil {
+		t.Fatalf("failed to init placement syncer: %v", err)
+	}
 	_, err := syncer.Sync(ctx)
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
@@ -78,18 +79,19 @@ func TestPlacementSyncer_SyncResourceProviders(t *testing.T) {
 	testDB := db.DB{DbMap: dbEnv.DbMap}
 	defer dbEnv.Close()
 	mon := datasources.Monitor{}
-	pc := &testlibKeystone.MockKeystoneClient{}
 	conf := v1alpha1.PlacementDatasource{Type: v1alpha1.PlacementDatasourceTypeResourceProviders}
 
 	syncer := &PlacementSyncer{
 		DB:   testDB,
 		Mon:  mon,
 		Conf: conf,
-		API:  NewPlacementAPI(mon, pc, conf),
+		API:  &mockPlacementAPI{},
 	}
-	syncer.API = &mockPlacementAPI{}
 
 	ctx := t.Context()
+	if err := syncer.Init(ctx); err != nil {
+		t.Fatalf("failed to init placement syncer: %v", err)
+	}
 	n, err := syncer.SyncResourceProviders(ctx)
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
@@ -104,7 +106,6 @@ func TestPlacementSyncer_SyncTraits(t *testing.T) {
 	testDB := db.DB{DbMap: dbEnv.DbMap}
 	defer dbEnv.Close()
 	mon := datasources.Monitor{}
-	pc := &testlibKeystone.MockKeystoneClient{}
 	conf := v1alpha1.PlacementDatasource{Type: v1alpha1.PlacementDatasourceTypeResourceProviderTraits}
 
 	rps := []ResourceProvider{{UUID: "1", Name: "rp1"}}
@@ -120,9 +121,12 @@ func TestPlacementSyncer_SyncTraits(t *testing.T) {
 		DB:   testDB,
 		Mon:  mon,
 		Conf: conf,
-		API:  NewPlacementAPI(mon, pc, conf),
+		API:  &mockPlacementAPI{},
+	}
+
+	if err := syncer.Init(t.Context()); err != nil {
+		t.Fatalf("failed to init placement syncer: %v", err)
 	}
-	syncer.API = &mockPlacementAPI{}
 
 	ctx := t.Context()
 	// First, we need to sync resource providers to have them in the DB.
@@ -140,7 +144,6 @@ func TestPlacementSyncer_SyncInventoryUsages(t *testing.T) {
 	testDB := db.DB{DbMap: dbEnv.DbMap}
 	defer dbEnv.Close()
 	mon := datasources.Monitor{}
-	pc := &testlibKeystone.MockKeystoneClient{}
 	conf := v1alpha1.PlacementDatasource{Type: v1alpha1.PlacementDatasourceTypeResourceProviderInventoryUsages}
 
 	rps := []ResourceProvider{{UUID: "1", Name: "rp1"}}
@@ -156,9 +159,12 @@ func TestPlacementSyncer_SyncInventoryUsages(t *testing.T) {
 		DB:   testDB,
 		Mon:  mon,
 		Conf: conf,
-		API:  NewPlacementAPI(mon, pc, conf),
+		API:  &mockPlacementAPI{},
+	}
+
+	if err := syncer.Init(t.Context()); err != nil {
+		t.Fatalf("failed to init placement syncer: %v", err)
 	}
-	syncer.API = &mockPlacementAPI{}
 
 	ctx := t.Context()
 	n, err := syncer.SyncInventoryUsages(ctx)

internal/knowledge/db/db.go

@@ -142,14 +142,16 @@ func (d *DB) SelectTimed(group string, i any, query string, args ...any) ([]any,
 func (d *DB) CreateTable(table ...*gorp.TableMap) error {
 	tx, err := d.Begin()
 	if err != nil {
-		slog.Error("failed to begin transaction", "error", err)
-		return tx.Rollback()
+		return fmt.Errorf("failed to begin transaction: %w", err)
 	}
 	for _, t := range table {
 		slog.Info("creating table if exists", "table", t.TableName)
 		sql := t.SqlForCreate(true) // true means to add IF NOT EXISTS
 		if _, err := tx.Exec(sql); err != nil {
-			return tx.Rollback()
+			if rbErr := tx.Rollback(); rbErr != nil {
+				slog.Error("failed to rollback transaction", "error", rbErr)
+			}
+			return fmt.Errorf("failed to create table %s: %w", t.TableName, err)
 		}
 	}
 	return tx.Commit()
@@ -201,20 +203,22 @@ func ReplaceAll[T Table](db DB, objs ...T) error {
 	tableName := model.TableName()
 	tx, err := db.Begin()
 	if err != nil {
-		slog.Error("failed to begin transaction", "error", err)
-		return tx.Rollback()
+		return fmt.Errorf("failed to begin transaction: %w", err)
 	}
 	if _, err = tx.Exec("DELETE FROM " + tableName); err != nil {
-		slog.Error("failed to delete old objects", "tableName", tableName, "error", err)
-		return tx.Rollback()
+		if rbErr := tx.Rollback(); rbErr != nil {
+			slog.Error("failed to rollback transaction", "error", rbErr)
+		}
+		return fmt.Errorf("failed to delete old objects from %s: %w", tableName, err)
 	}
 	if err = BulkInsert(tx, db, objs...); err != nil {
-		slog.Error("failed to insert new objects", "tableName", tableName, "error", err)
-		return tx.Rollback()
+		if rbErr := tx.Rollback(); rbErr != nil {
+			slog.Error("failed to rollback transaction", "error", rbErr)
+		}
+		return fmt.Errorf("failed to insert new objects into %s: %w", tableName, err)
 	}
 	if err = tx.Commit(); err != nil {
-		slog.Error("failed to commit transaction", "error", err)
-		return err
+		return fmt.Errorf("failed to commit transaction: %w", err)
 	}
 	return nil
 }

internal/knowledge/db/db_test.go

@@ -4,6 +4,7 @@
 package db
 
 import (
+	"strings"
 	"testing"
 	"time"
 
@@ -119,6 +120,56 @@ func TestReplaceAll(t *testing.T) {
 	}
 }
 
+func TestReplaceAllWithDuplicateKeys(t *testing.T) {
+	dbEnv := testlibDB.SetupDBEnv(t)
+	db := DB{DbMap: dbEnv.DbMap}
+	defer dbEnv.Close()
+
+	table := db.AddTable(MockTable{})
+	if err := db.CreateTable(table); err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+
+	// Insert initial records.
+	for _, record := range []MockTable{{ID: 1, Name: "old"}} {
+		if err := db.Insert(&record); err != nil {
+			t.Fatalf("expected no error, got %v", err)
+		}
+	}
+
+	// ReplaceAll with duplicate primary keys in the input.
+	dupes := []MockTable{
+		{ID: 10, Name: "first"},
+		{ID: 10, Name: "duplicate"},
+		{ID: 20, Name: "other"},
+	}
+	err := ReplaceAll(db, dupes...)
+
+	// The insert should fail because of the duplicate key.
+	if err == nil {
+		t.Fatal("expected an error for duplicate keys, got nil")
+	}
+	if !strings.Contains(err.Error(), "failed to insert") {
+		t.Fatalf("expected insert error, got: %v", err)
+	}
+
+	// The old data must still be intact because the transaction was rolled back.
+	var count int
+	if err := db.SelectOne(&count, "SELECT COUNT(*) FROM mock_table"); err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+	if count != 1 {
+		t.Fatalf("expected 1 record (old data intact after rollback), got %d", count)
+	}
+	var name string
+	if err := db.SelectOne(&name, "SELECT name FROM mock_table WHERE id = 1"); err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+	if name != "old" {
+		t.Fatalf("expected old data to be intact, got name=%q", name)
+	}
+}
+
 // Test all sorts of data types.
 type BulkMockTable struct {
 	A int        `db:"a,primarykey"`