diff --git a/cmd/praefect/subcmd.go b/cmd/praefect/subcmd.go index a85f0b7721fd567e4f1f0819331b0278285ccd88..83afe3d52347dece4541584f27544647bd02414f 100644 --- a/cmd/praefect/subcmd.go +++ b/cmd/praefect/subcmd.go @@ -16,6 +16,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore/glsql" + "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore/migrations" "google.golang.org/grpc" ) @@ -125,7 +126,7 @@ func (s *sqlMigrateSubcommand) Exec(flags *flag.FlagSet, conf config.Config) err } defer clean() - n, err := glsql.Migrate(db, s.ignoreUnknown) + n, err := glsql.Migrate(db, s.ignoreUnknown, migrations.All()) if err != nil { return fmt.Errorf("%s: fail: %v", subCmd, err) } diff --git a/internal/praefect/datastore/glsql/postgres.go b/internal/praefect/datastore/glsql/postgres.go index 4b5efcd86a33eb0092eec758fe715f095b6f826a..28a80306ab4ac5f926111be7201e3bae8aad12b5 100644 --- a/internal/praefect/datastore/glsql/postgres.go +++ b/internal/praefect/datastore/glsql/postgres.go @@ -28,14 +28,14 @@ func OpenDB(conf config.DB) (*sql.DB, error) { } // Migrate will apply all pending SQL migrations. -func Migrate(db *sql.DB, ignoreUnknown bool) (int, error) { +func Migrate(db *sql.DB, ignoreUnknown bool, mgs []*migrate.Migration) (int, error) { migrationSet := migrate.MigrationSet{ IgnoreUnknown: ignoreUnknown, TableName: migrations.MigrationTableName, } migrationSource := &migrate.MemoryMigrationSource{ - Migrations: migrations.All(), + Migrations: mgs, } return migrationSet.Exec(db, "postgres", migrationSource, migrate.Up) diff --git a/internal/praefect/datastore/glsql/postgres_test.go b/internal/praefect/datastore/glsql/postgres_test.go index b91db2b58b745aacafbf6b9a8ded0b6025b8c978..26d3d6482ffe4fdc02924bd3ced8cdf921a996ee 100644 --- a/internal/praefect/datastore/glsql/postgres_test.go +++ b/internal/praefect/datastore/glsql/postgres_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore/migrations" ) func TestOpenDB(t *testing.T) { @@ -63,3 +64,54 @@ func TestScanAll(t *testing.T) { require.NoError(t, ScanAll(emptyRows, ¬hing)) require.Equal(t, ([]uint64)(nil), nothing.Values()) } + +func TestAdditionalTestMigrationsWereApplied(t *testing.T) { + t.Parallel() + + const dbName = "migrations_verification" + + postgresDBCfg := GetDBConfig(t, "postgres") + postgresDB := requireSQLOpen(t, postgresDBCfg, true) + defer func() { + require.NoErrorf(t, postgresDB.Close(), "release connection to the %q database", postgresDBCfg.DBName) + }() + + _, err := postgresDB.Exec("DROP DATABASE IF EXISTS " + dbName + "") + require.NoErrorf(t, err, `failed to drop %q database`, dbName) + _, err = postgresDB.Exec("CREATE DATABASE " + dbName + " WITH ENCODING 'UTF8'") + require.NoErrorf(t, err, `failed to create %q database`, dbName) + + migrationsDBCfg := GetDBConfig(t, dbName) + migrationsDB := requireSQLOpen(t, migrationsDBCfg, true) + defer func() { + require.NoErrorf(t, migrationsDB.Close(), "release connection to the %q database", migrationsDBCfg.DBName) + }() + + migrationsList := append(migrations.All(), testDataMigrations()...) + _, err = Migrate(migrationsDB, false, migrationsList) + require.NoError(t, err) + + var ms StringProvider + rows, err := migrationsDB.Query(`SELECT id FROM ` + migrations.MigrationTableName) + require.NoError(t, err) + require.NoError(t, ScanAll(rows, &ms)) + require.Len(t, ms.Values(), len(migrationsList), "sanity check to verify all migrations were executed") + var additionalMigrations []string + for _, applied := range ms.Values() { + var found bool + for _, mig := range migrations.All() { + if mig.Id == applied { + found = true + break + } + } + if !found { + additionalMigrations = append(additionalMigrations, applied) + } + } + require.ElementsMatch(t, additionalMigrations, []string{ + "10000000000000_helpers", + "20210906145020_artificial_repositories", + "20210906145022_artificial_repositories_cleanup", + }, "sanity check to verify all additional test migrations were applied") +} diff --git a/internal/praefect/datastore/glsql/testing.go b/internal/praefect/datastore/glsql/testing.go index 915576985658fba446dcad217e3e78e74ad422c3..d5c35efe8a1a5f4ebdc3c71d2ebb792a3bd549e5 100644 --- a/internal/praefect/datastore/glsql/testing.go +++ b/internal/praefect/datastore/glsql/testing.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/config" + "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/datastore/migrations" ) const ( @@ -215,7 +216,7 @@ func initPraefectTestDB(t testing.TB, database string) *sql.DB { require.NoErrorf(t, templateDB.Close(), "release connection to the %q database", templateDBConf.DBName) }() - if _, err := Migrate(templateDB, false); err != nil { + if _, err := Migrate(templateDB, false, migrations.All()); err != nil { // If database has unknown migration we try to re-create template database with // current migration. It may be caused by other code changes done in another branch. if pErr := (*migrate.PlanError)(nil); errors.As(err, &pErr) { @@ -231,7 +232,7 @@ func initPraefectTestDB(t testing.TB, database string) *sql.DB { defer func() { require.NoErrorf(t, remigrateTemplateDB.Close(), "release connection to the %q database", templateDBConf.DBName) }() - _, err = Migrate(remigrateTemplateDB, false) + _, err = Migrate(remigrateTemplateDB, false, migrations.All()) require.NoErrorf(t, err, "failed to run database migration on %q", praefectTemplateDatabase) } else { require.NoErrorf(t, err, "failed to run database migration on %q", praefectTemplateDatabase) @@ -330,3 +331,86 @@ func getDatabaseEnvironment(t testing.TB) map[string]string { return databaseEnv } + +// testDataMigrations function should be used to include additional migration scripts into the +// existing set of migrations. All migrations applied in order. The ordering is done via numeric +// prefix of the migration. To insert migration in between some existing migrations you should +// include it with the prefix that will be greater that the lowest and lesser than the upper one. +func testDataMigrations() []*migrate.Migration { + return []*migrate.Migration{ + { + Id: "10000000000000_helpers", + Up: []string{ + // random_hex_string generates a random HEX string with requested length. + `-- +migrate StatementBegin + CREATE OR REPLACE FUNCTION random_hex_string(length INTEGER) RETURNS TEXT AS + $$ + DECLARE + chars TEXT[] := '{0,1,2,3,4,5,6,7,8,9,a,b,c,d,e,f}'; + result TEXT := ''; + i INTEGER := 0; + BEGIN + IF length < 0 THEN + RAISE EXCEPTION 'Given length cannot be less than 0'; + END IF; + FOR i IN 1..length LOOP + result := result || chars[1+RANDOM()*(ARRAY_LENGTH(chars, 1)-1)]; + END LOOP; + RETURN result; + END; + $$ LANGUAGE plpgsql`, + + // repository_relative_path generates a relative path for the repository. + // It has a standard format: @hashed/ab/cd/abcd000000000000000000000000000000000000000000000000000000000000.git + `-- +migrate StatementBegin + CREATE OR REPLACE FUNCTION repository_relative_path() RETURNS TEXT AS + $$ + DECLARE + result TEXT; + BEGIN + SELECT CONCAT_WS('/', '@hashed', SUBSTR(path, 1, 2), SUBSTR(path, 3, 2), path || '.git') INTO result + FROM (SELECT random_hex_string(64) AS path) t; + RETURN result; + END; + $$ LANGUAGE plpgsql + -- +migrate StatementEnd`, + }, + }, + // Verify 20210906145021_link_repository_id migration runs without issues. + { + // Applied before 20210906145021_link_repository_id + // Migration populates database tables with artificial data. + // It is needed to check execution of the 20210906145021_link_repository_id migration. + Id: "20210906145020_artificial_repositories", + Up: []string{ + `ALTER TABLE storage_repositories DISABLE TRIGGER notify_on_insert`, + `INSERT INTO storage_repositories (virtual_storage, relative_path, storage, generation) + SELECT virtual_storages.name, repositories.relative_path, storages.name, 1 + FROM (SELECT UNNEST(ARRAY['artificial-praefect-1','artificial-praefect-2']) AS name) virtual_storages + CROSS JOIN + (SELECT UNNEST(ARRAY['artificial-gitaly-1','artificial-gitaly-2','artificial-gitaly-3']) AS name) storages + CROSS JOIN + (SELECT generate_series(1,300), repository_relative_path() AS relative_path) repositories`, + + `INSERT INTO repositories (virtual_storage, relative_path, generation) + SELECT DISTINCT virtual_storage, relative_path, 1 + FROM storage_repositories`, + `ALTER TABLE storage_repositories ENABLE TRIGGER notify_on_insert`, + }, + }, + { + // Applied after 20210906145021_link_repository_id + // Migration removes artificial data from the repository that was used to verify migration. + Id: "20210906145022_artificial_repositories_cleanup", + Up: []string{ + `ALTER TABLE storage_repositories DISABLE TRIGGER notify_on_delete`, + `DELETE FROM storage_repositories WHERE virtual_storage IN ('artificial-praefect-1','artificial-praefect-2')`, + `ALTER TABLE storage_repositories ENABLE TRIGGER notify_on_delete`, + + `ALTER TABLE repositories DISABLE TRIGGER notify_on_delete`, + `DELETE FROM repositories WHERE virtual_storage IN ('artificial-praefect-1','artificial-praefect-2')`, + `ALTER TABLE repositories ENABLE TRIGGER notify_on_delete`, + }, + }, + } +}