Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BCF-3276: refactor migrations to use goose provider #13678

Merged
merged 5 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silent-cups-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": minor
---

#internal refactor goose migrations to use provider
5 changes: 3 additions & 2 deletions core/scripts/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ require (
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mattn/go-runewidth v0.0.14 // indirect
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
github.com/mfridman/interpolate v0.0.2 // indirect
github.com/mimoo/StrobeGo v0.0.0-20210601165009-122bf33a46e0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
Expand All @@ -254,7 +255,7 @@ require (
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
github.com/pressly/goose/v3 v3.16.0 // indirect
github.com/pressly/goose/v3 v3.21.1 // indirect
github.com/prometheus/client_model v0.5.0 // indirect
github.com/prometheus/common v0.45.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect
Expand Down Expand Up @@ -330,7 +331,7 @@ require (
go.uber.org/zap v1.26.0 // indirect
golang.org/x/arch v0.7.0 // indirect
golang.org/x/crypto v0.24.0 // indirect
golang.org/x/exp v0.0.0-20240213143201-ec583247a57a // indirect
golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8 // indirect
golang.org/x/mod v0.17.0 // indirect
golang.org/x/net v0.25.0 // indirect
golang.org/x/oauth2 v0.20.0 // indirect
Expand Down
95 changes: 24 additions & 71 deletions core/scripts/go.sum

Large diffs are not rendered by default.

90 changes: 70 additions & 20 deletions core/store/migrate/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import (
"database/sql"
"embed"
"fmt"
"io/fs"
"os"
"strconv"
"strings"

pkgerrors "github.com/pkg/errors"
"github.com/pressly/goose/v3"
"github.com/pressly/goose/v3/database"
"gopkg.in/guregu/null.v4"

"github.com/smartcontractkit/chainlink-common/pkg/sqlutil"
Expand All @@ -25,20 +27,55 @@ var embedMigrations embed.FS

const MIGRATIONS_DIR string = "migrations"

func init() {
goose.SetBaseFS(embedMigrations)
goose.SetSequential(true)
goose.SetTableName("goose_migrations")
func NewProvider(ctx context.Context, db *sql.DB) (*goose.Provider, error) {
store, err := database.NewStore(goose.DialectPostgres, "goose_migrations")
if err != nil {
return nil, err
}

goMigrations := []*goose.Migration{
migrations.Migration36,
migrations.Migration54,
migrations.Migration56,
migrations.Migration195,
}

logMigrations := os.Getenv("CL_LOG_SQL_MIGRATIONS")
verbose, _ := strconv.ParseBool(logMigrations)
goose.SetVerbose(verbose)

fys, err := fs.Sub(embedMigrations, MIGRATIONS_DIR)
if err != nil {
return nil, fmt.Errorf("failed to get sub filesystem for embedded migration dir: %w", err)
}
// hack to work around global go migrations
// https: //github.com/pressly/goose/issues/782
goose.ResetGlobalMigrations()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any concerns of the usage of this hack?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, goose has already fixed the issue and it will be in a new release soon

p, err := goose.NewProvider("", db, fys,
goose.WithStore(store),
goose.WithGoMigrations(goMigrations...),
goose.WithVerbose(verbose))
if err != nil {
return nil, fmt.Errorf("failed to create goose provider: %w", err)
}

err = ensureMigrated(ctx, db, p, store.Tablename())
if err != nil {
return nil, err
}

return p, nil
}

// Ensure we migrated from v1 migrations to goose_migrations
func ensureMigrated(ctx context.Context, db *sql.DB) error {
// TODO remove this for v3
func ensureMigrated(ctx context.Context, db *sql.DB, p *goose.Provider, providerTableName string) error {
todo, err := p.HasPending(ctx)
if !todo && err == nil {
return nil
}
sqlxDB := pg.WrapDbWithSqlx(db)
var names []string
err := sqlxDB.SelectContext(ctx, &names, `SELECT id FROM migrations`)
err = sqlxDB.SelectContext(ctx, &names, `SELECT id FROM migrations`)
if err != nil {
// already migrated
return nil
Expand All @@ -63,13 +100,14 @@ func ensureMigrated(ctx context.Context, db *sql.DB) error {
}

// ensure a goose migrations table exists with it's initial v0
if _, err = goose.GetDBVersionContext(ctx, db); err != nil {
if _, err = p.GetDBVersion(ctx); err != nil {
return err
}

// insert records for existing migrations
//nolint
sql := fmt.Sprintf(`INSERT INTO %s (version_id, is_applied) VALUES ($1, true);`, goose.TableName())

sql := fmt.Sprintf(`INSERT INTO %s (version_id, is_applied) VALUES ($1, true);`, providerTableName)
return sqlutil.TransactDataSource(ctx, sqlxDB, nil, func(tx sqlutil.DataSource) error {
for _, name := range names {
var id int64
Expand Down Expand Up @@ -100,36 +138,48 @@ func ensureMigrated(ctx context.Context, db *sql.DB) error {
}

func Migrate(ctx context.Context, db *sql.DB) error {
if err := ensureMigrated(ctx, db); err != nil {
provider, err := NewProvider(ctx, db)
if err != nil {
return err
}
// WithAllowMissing is necessary when upgrading from 0.10.14 since it
// includes out-of-order migrations
return goose.Up(db, MIGRATIONS_DIR, goose.WithAllowMissing())
_, err = provider.Up(ctx)
return err
}

func Rollback(ctx context.Context, db *sql.DB, version null.Int) error {
if err := ensureMigrated(ctx, db); err != nil {
provider, err := NewProvider(ctx, db)
if err != nil {
return err
}
if version.Valid {
return goose.DownTo(db, MIGRATIONS_DIR, version.Int64)
_, err = provider.DownTo(ctx, version.Int64)
} else {
_, err = provider.Down(ctx)
}
return goose.Down(db, MIGRATIONS_DIR)
return err
}

func Current(ctx context.Context, db *sql.DB) (int64, error) {
if err := ensureMigrated(ctx, db); err != nil {
provider, err := NewProvider(ctx, db)
if err != nil {
return -1, err
}
return goose.EnsureDBVersion(db)
return provider.GetDBVersion(ctx)
}

func Status(ctx context.Context, db *sql.DB) error {
if err := ensureMigrated(ctx, db); err != nil {
provider, err := NewProvider(ctx, db)
if err != nil {
return err
}
migrations, err := provider.Status(ctx)
if err != nil {
return err
}
return goose.Status(db, MIGRATIONS_DIR)
for _, m := range migrations {
fmt.Printf("version:%d, path:%s, type:%s, state:%s, appliedAt: %s \n", m.Source.Version, m.Source.Path, m.Source.Type, m.State, m.AppliedAt.String())
}
return nil
}

func Create(db *sql.DB, name, migrationType string) error {
Expand Down
50 changes: 34 additions & 16 deletions core/store/migrate/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/google/uuid"
"github.com/lib/pq"
"github.com/pressly/goose/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/guregu/null.v4"

Expand All @@ -29,8 +30,6 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/store/models"
)

var migrationDir = "migrations"

type OffchainReporting2OracleSpec100 struct {
ID int32 `toml:"-"`
ContractID string `toml:"contractID"`
Expand Down Expand Up @@ -72,8 +71,11 @@ func getOCR2Spec100() OffchainReporting2OracleSpec100 {
func TestMigrate_0100_BootstrapConfigs(t *testing.T) {
cfg, db := heavyweight.FullTestDBEmptyV2(t, nil)
lggr := logger.TestLogger(t)
err := goose.UpTo(db.DB, migrationDir, 99)
p, err := migrate.NewProvider(testutils.Context(t), db.DB)
require.NoError(t, err)
results, err := p.UpTo(testutils.Context(t), 99)
require.NoError(t, err)
assert.Len(t, results, 99)

pipelineORM := pipeline.NewORM(db, lggr, cfg.JobPipeline().MaxSuccessfulRuns())
ctx := testutils.Context(t)
Expand Down Expand Up @@ -227,7 +229,7 @@ func TestMigrate_0100_BootstrapConfigs(t *testing.T) {
require.NoError(t, err)

// Migrate up
err = goose.UpByOne(db.DB, migrationDir)
_, err = p.UpByOne(ctx)
require.NoError(t, err)

var bootstrapSpecs []job.BootstrapSpec
Expand Down Expand Up @@ -282,7 +284,7 @@ func TestMigrate_0100_BootstrapConfigs(t *testing.T) {
require.Equal(t, 1, count)

// Migrate down
err = goose.Down(db.DB, migrationDir)
_, err = p.Down(ctx)
require.NoError(t, err)

var oldJobs []Job
Expand Down Expand Up @@ -340,8 +342,12 @@ ON jobs.offchainreporting2_oracle_spec_id = ocr2.id`

func TestMigrate_101_GenericOCR2(t *testing.T) {
_, db := heavyweight.FullTestDBEmptyV2(t, nil)
err := goose.UpTo(db.DB, migrationDir, 100)
ctx := testutils.Context(t)
p, err := migrate.NewProvider(ctx, db.DB)
require.NoError(t, err)
results, err := p.UpTo(ctx, 100)
require.NoError(t, err)
assert.Len(t, results, 100)

sql := `INSERT INTO offchainreporting2_oracle_specs (id, contract_id, relay, relay_config, p2p_bootstrap_peers, ocr_key_bundle_id, transmitter_id,
blockchain_timeout, contract_config_tracker_poll_interval, contract_config_confirmations, juels_per_fee_coin_pipeline,
Expand All @@ -356,7 +362,7 @@ func TestMigrate_101_GenericOCR2(t *testing.T) {
_, err = db.NamedExec(sql, spec)
require.NoError(t, err)

err = goose.UpByOne(db.DB, migrationDir)
_, err = p.UpByOne(ctx)
require.NoError(t, err)

type PluginValues struct {
Expand All @@ -373,7 +379,7 @@ func TestMigrate_101_GenericOCR2(t *testing.T) {
require.Equal(t, types.Median, pluginValues.PluginType)
require.Equal(t, job.JSONConfig{"juelsPerFeeCoinSource": spec.JuelsPerFeeCoinPipeline}, pluginValues.PluginConfig)

err = goose.Down(db.DB, migrationDir)
_, err = p.Down(ctx)
require.NoError(t, err)

sql = `SELECT plugin_type, plugin_config FROM offchainreporting2_oracle_specs`
Expand All @@ -390,8 +396,12 @@ func TestMigrate_101_GenericOCR2(t *testing.T) {
func TestMigrate(t *testing.T) {
ctx := testutils.Context(t)
_, db := heavyweight.FullTestDBEmptyV2(t, nil)
err := goose.UpTo(db.DB, migrationDir, 100)

p, err := migrate.NewProvider(ctx, db.DB)
require.NoError(t, err)
results, err := p.UpTo(ctx, 100)
require.NoError(t, err)
assert.Len(t, results, 100)

err = migrate.Status(ctx, db.DB)
require.NoError(t, err)
Expand Down Expand Up @@ -443,8 +453,11 @@ func TestDatabaseBackFillWithMigration202(t *testing.T) {
_, db := heavyweight.FullTestDBEmptyV2(t, nil)
ctx := testutils.Context(t)

err := goose.UpTo(db.DB, migrationDir, 201)
p, err := migrate.NewProvider(ctx, db.DB)
require.NoError(t, err)
results, err := p.UpTo(ctx, 201)
require.NoError(t, err)
assert.Len(t, results, 201)

simulatedOrm := logpoller.NewORM(testutils.SimulatedChainID, db, logger.TestLogger(t))
require.NoError(t, simulatedOrm.InsertBlock(ctx, testutils.Random32Byte(), 10, time.Now(), 0), err)
Expand All @@ -458,7 +471,7 @@ func TestDatabaseBackFillWithMigration202(t *testing.T) {
klaytnOrm := logpoller.NewORM(big.NewInt(int64(1001)), db, logger.TestLogger(t))
require.NoError(t, klaytnOrm.InsertBlock(ctx, testutils.Random32Byte(), 100, time.Now(), 0), err)

err = goose.UpTo(db.DB, migrationDir, 202)
_, err = p.UpTo(ctx, 202)
require.NoError(t, err)

tests := []struct {
Expand Down Expand Up @@ -530,8 +543,10 @@ func TestNoTriggers(t *testing.T) {
assert_num_triggers(0)

// version prior to removal of all triggers
v := 217
err := goose.UpTo(db.DB, migrationDir, int64(v))
v := int64(217)
p, err := migrate.NewProvider(testutils.Context(t), db.DB)
require.NoError(t, err)
_, err = p.UpTo(testutils.Context(t), v)
require.NoError(t, err)
assert_num_triggers(1)
}
Expand All @@ -547,8 +562,11 @@ func BenchmarkBackfillingRecordsWithMigration202(b *testing.B) {
goose.SetLogger(goose.NopLogger())
_, db := heavyweight.FullTestDBEmptyV2(b, nil)

err := goose.UpTo(db.DB, migrationDir, previousMigration)
p, err := migrate.NewProvider(ctx, db.DB)
require.NoError(b, err)
results, err := p.UpTo(ctx, previousMigration)
require.NoError(b, err)
assert.Len(b, results, int(previousMigration))

for j := 0; j < chainCount; j++ {
// Insert 100_000 block to database, can't do all at once, so batching by 10k
Expand Down Expand Up @@ -586,12 +604,12 @@ func BenchmarkBackfillingRecordsWithMigration202(b *testing.B) {
// Repeat 1-3
for i := 0; i < b.N; i++ {
b.StartTimer()
err = goose.UpTo(db.DB, migrationDir, backfillMigration)
_, err = p.UpTo(ctx, backfillMigration)
require.NoError(b, err)
b.StopTimer()

// Cleanup
err = goose.DownTo(db.DB, migrationDir, previousMigration)
_, err = p.DownTo(ctx, previousMigration)
require.NoError(b, err)

_, err = db.ExecContext(ctx, `
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ CREATE TABLE feeds_managers (
updated_at timestamp with time zone NOT NULL
);
-- +goose Down
DROP TABLE feeds_managers
DROP TABLE feeds_managers;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix a linting issue found by the new version of goose

6 changes: 2 additions & 4 deletions core/store/migrate/migrations/0036_external_job_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ import (
"github.com/pressly/goose/v3"
)

func init() {
goose.AddMigrationContext(Up36, Down36)
}

const (
up36_1 = `
ALTER TABLE direct_request_specs DROP COLUMN on_chain_job_spec_id;
Expand Down Expand Up @@ -79,3 +75,5 @@ func Down36(ctx context.Context, tx *sql.Tx) error {
}
return nil
}

var Migration36 = goose.NewGoMigration(36, &goose.GoFunc{RunTx: Up36}, &goose.GoFunc{RunTx: Down36})
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ ALTER TABLE log_broadcasts RENAME COLUMN job_id_v2 TO job_id;
ALTER TABLE job_spec_errors_v2 RENAME TO job_spec_errors;
`

func init() {
goose.AddMigrationContext(Up54, Down54)
}

type queryer interface {
QueryRowContext(ctx context.Context, query string, args ...interface{}) *sql.Row
}
Expand Down Expand Up @@ -63,3 +59,5 @@ func CheckNoLegacyJobs(ctx context.Context, ds queryer) error {
}
return nil
}

var Migration54 = goose.NewGoMigration(54, &goose.GoFunc{RunTx: Up54}, &goose.GoFunc{RunTx: Down54})
Loading
Loading