Make migrations idempotent

Previously migrations failed if they were run on an already migrated database.
This made the connector fail if it was ever restarted.

Author: cevian

helm-chart/Chart.yaml

@@ -1,4 +1,4 @@
 name: timescale-prometheus
-version: 0.1.0-alpha.2
+version: 0.1.0-alpha.3
 apiVersion: v2
 description: Timescale Prometheus Connector deployment

pkg/pgmodel/end_to_end_tests/migrate_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/jackc/pgx/v4/pgxpool"
+	"github.com/timescale/timescale-prometheus/pkg/internal/testhelpers"
 )
 
 const (
@@ -34,3 +35,13 @@ func TestMigrate(t *testing.T) {
 
 	})
 }
+
+func TestMigrateTwice(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+	testhelpers.WithDB(t, *testDatabase, testhelpers.NoSuperuser, func(db *pgxpool.Pool, t testing.TB, connectURL string) {
+		performMigrate(t, *testDatabase, connectURL)
+		performMigrate(t, *testDatabase, connectURL)
+	})
+}

pkg/pgmodel/migrate.go

@@ -22,7 +22,7 @@ import (
 
 const (
 	timescaleInstall = "CREATE EXTENSION IF NOT EXISTS timescaledb WITH SCHEMA public;"
-	extensionInstall = "CREATE EXTENSION timescale_prometheus_extra WITH SCHEMA %s;"
+	extensionInstall = "CREATE EXTENSION IF NOT EXISTS timescale_prometheus_extra WITH SCHEMA %s;"
 )
 
 type mySrc struct {
@@ -71,7 +71,7 @@ func (t *mySrc) ReadDown(version uint) (r io.ReadCloser, identifier string, err
 }
 
 // Migrate performs a database migration to the latest version
-func Migrate(db *sql.DB) error {
+func Migrate(db *sql.DB) (err error) {
 	// The migration table will be put in the public schema not in any of our schema because we never want to drop it and
 	// our scripts and our last down script drops our shemas
 	driver, err := postgres.WithInstance(db, &postgres.Config{MigrationsTable: "prom_schema_migrations"})
@@ -94,21 +94,35 @@ func Migrate(db *sql.DB) error {
 	if err != nil {
 		return err
 	}
+	defer func() {
+		sourceErr, databaseErr := m.Close()
+		//don't override error if already set
+		if err != nil {
+			return
+		}
+		if sourceErr != nil {
+			err = sourceErr
+			return
+		}
+		if databaseErr != nil {
+			err = databaseErr
+			return
+		}
+	}()
 
 	err = m.Up()
-	if err == nil {
-		_, extErr := db.Exec(fmt.Sprintf(extensionInstall, extSchema))
-		if extErr != nil {
-			log.Warn("msg", "timescale_prometheus_extra extension not installed", "cause", extErr)
-		}
+	//ignore no change errors as we want this idempotent. Being up to date is not a bad thing.
+	if err == migrate.ErrNoChange {
+		err = nil
 	}
-
-	sErr, dErr := m.Close()
-	if sErr != nil {
-		return sErr
+	if err != nil {
+		return err
 	}
-	if dErr != nil {
-		return dErr
+
+	_, extErr := db.Exec(fmt.Sprintf(extensionInstall, extSchema))
+	if extErr != nil {
+		log.Warn("msg", "timescale_prometheus_extra extension not installed", "cause", extErr)
 	}
-	return err
+
+	return nil
 }