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.

Sign up for GitHub

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

Jump to bottom

Integrate a migration tool in tink #295

Closed
gianarb opened this issue Sep 17, 2020 · 4 comments · Fixed by #296 or #313
Closed

Integrate a migration tool in tink #295

gianarb opened this issue Sep 17, 2020 · 4 comments · Fixed by #296 or #313
Assignees
@gianarb
Labels
area/setup Issue related to tinkerbell setup kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/M estimate of the amount of work to address the issue

Comments

@gianarb
Copy link
Contributor

gianarb commented Sep 17, 2020 •
edited by gauravgahlot
Loading

Tink uses Postgres as datastore.

There are two things I would like to achieve here:

  1. We need a way to migrate our database schema as part of Event driven implementation #294 . We need to add an event table at least.
  2. Right now the database initialization is applied via docker-compose. I would like to have a more generic mechanism and even better if it can be managed by tink itself.

I explored a couple of libraries:

  • https://github.com/amacneil/dbmate
    • 👍 their readme makes a comparison with the other libraries and dbmate looks like the best one ever!
    • 👎 their readme makes a comparison with the other libraries and dbmate looks like the best one ever!
    • 👎 it is in Go, so we can embed the library, but there is no documentation about how to do it
  • https://github.com/golang-migrate/migrate
    • 👍 it has more starts that all the others and community is active for what I can tell
    • 👍 Packet used it internally already
    • 👍 It is designed with embeddability in mind as you can see from their README.
    • 👎 dbmate's README says that dbmate is cooler!
  • https://github.com/kyleconroy/sqlc
    • 👎 It does not really do the migration, it is more like code generation from SQL.
    • 👍 I love this library but its not what I am looking for now, it works with dbmate and migrate. We should keep it in mind as a possible way to generate some of the code we write manually.

I think migrate is what I am looking for. As I said the migration will be applied at every boot (it is idempotent on its own, so if there are not migration to apply, we are good).

I will start with the fil source but I am investing the possibility to use a custom source (source is where migrations are read from) because I would like to have the migration file written in *.go and embedded in the binary itself, to simplify distribution.
@gianarb gianarb self-assigned this Sep 17, 2020
@gianarb gianarb added area/setup Issue related to tinkerbell setup kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/M estimate of the amount of work to address the issue labels Sep 17, 2020
@thebsdbox
Copy link
Contributor

I'm a little confused with your 👍 👎 ranking, but if we're already using migrate and it is relatively active then it seems like the path of least resistance 👍

@rugwirobaker
Copy link

rugwirobaker commented Sep 17, 2020 •
edited
Loading

I think https://github.com/rubenv/sql-migrate is the most complete among them. It even gives you the ability to imbed your migrations using packr.

@gianarb gianarb mentioned this issue Sep 17, 2020
Closed
@Cbkhare
Copy link
Contributor

Cbkhare commented Sep 17, 2020 •
edited
Loading

I found this big list of Go Schema Migration tools with good comparisons. May be it can also help.

https://povilasv.me/go-schema-migration-tools/

Moreover, https://github.com/golang-migrate/migrate seems to be a good choice.

@gianarb
Copy link
Contributor Author

gianarb commented Sep 17, 2020 •
edited
Loading

@Cbkhare the article is good but published in PUBLISHED FEBRUARY 20TH, 2017. In Go it means FOREVER! eheh

@thebsdbox those are pros and cons based on my judgment

I am evaluating the lib @rugwirobaker shared because I like how easy it looks to embed

@gianarb gianarb mentioned this issue Sep 17, 2020
Merged
3 tasks
@mergify mergify bot closed this as completed in #296 Sep 28, 2020
mergify bot added a commit that referenced this issue Sep 28, 2020
@mergify
Apply migrations when tink-server boots (#296)
11bd870 
## Description

It will be nice to have the tink-server capable of applying migrations
if necessary. The idea to self-apply migrations will may look unusual
but it is an easy way to distribute software capable of migrating
database schema without having to run any external procedures.
## Why is this needed

Fixes: #295 

## How Has This Been Tested?

```console
$ docker run -d -it -e  POSTGRES_PASSWORD=tinkerbell -e POSTGRES_USER=tinkerbell -p 5432:5432 postgres
# PGPASSWORD=tinkerbell PGUSER=tinkerbell PGSSLMODE=disable PACKET_VERSION=v1 PACKET_ENV=ga ROLLBAR_TOKEN=ignored go run cmd/tink-server/main.go
{"level":"info","ts":1600772721.3009,"caller":"tink-server/main.go:30","msg":"starting version devel","service":"github.com/tinkerbell/tink"}
migrations  1
records  0
{"level":"info","ts":1600772721.309925,"caller":"tink-server/main.go:54","msg":"Your database schema is not up to date. Please apply migrations running tink-server with env var ONLY_MIGRATION set.","service":"github.com/tinkerbell/tink"}
{"level":"info","ts":1600772721.310011,"caller":"metrics/metrics.go:58","msg":"initializing label values","service":"github.com/tinkerbell/tink"}
```

As suggested you should run tink-server with ONLY_MIGRATION=true

```
$ ONLY_MIGRATION=true PGPASSWORD=tinkerbell PGUSER=tinkerbell PGSSLMODE=disable PACKET_VERSION=v1 PACKET_ENV=ga ROLLBAR_TOKEN=ignored go run cmd/tink-server/main.go
{"level":"info","ts":1600772853.779405,"caller":"tink-server/main.go:30","msg":"starting version devel","service":"github.com/tinkerbell/tink"}
{"level":"info","ts":1600772853.779805,"caller":"tink-server/main.go:40","msg":"Applying migrations. This process will end when migrations will take place.","service":"github.com/tinkerbell/tink"}
{"level":"info","ts":1600772853.819854,"caller":"tink-server/main.go:45","msg":"Running migrations if necessary","service":"github.com/tinkerbell/tink","num_applied_migrations":1}
```

## How are existing users impacted? What migration steps/scripts do we need?

They are not impacted by this change.

## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade
@gianarb gianarb mentioned this issue Oct 1, 2020
Merged
3 tasks
mergify bot added a commit that referenced this issue Oct 1, 2020
@mergify
Apply migrations when tink-server boots (#313)
a686336 
## Description

It will be nice to have the tink-server capable of applying migrations
if necessary. The idea to self-apply migrations will may look unusual
but it is an easy way to distribute software capable of migrating
database schema without having to run any external procedures.

```console
$ docker run -d -it -e  POSTGRES_PASSWORD=tinkerbell -e POSTGRES_USER=tinkerbell -p 5432:5432 postgres
# PGPASSWORD=tinkerbell PGUSER=tinkerbell PGSSLMODE=disable PACKET_VERSION=v1 PACKET_ENV=ga ROLLBAR_TOKEN=ignored go run cmd/tink-server/main.go
{"level":"info","ts":1600772721.3009,"caller":"tink-server/main.go:30","msg":"starting version devel","service":"github.com/tinkerbell/tink"}
migrations  1
records  0
{"level":"info","ts":1600772721.309925,"caller":"tink-server/main.go:54","msg":"Your database schema is not up to date. Please apply migrations running tink-server with env var ONLY_MIGRATION set.","service":"github.com/tinkerbell/tink"}
{"level":"info","ts":1600772721.310011,"caller":"metrics/metrics.go:58","msg":"initializing label values","service":"github.com/tinkerbell/tink"}
```

As suggested you should run tink-server with ONLY_MIGRATION=true

```
$ ONLY_MIGRATION=true PGPASSWORD=tinkerbell PGUSER=tinkerbell PGSSLMODE=disable PACKET_VERSION=v1 PACKET_ENV=ga ROLLBAR_TOKEN=ignored go run cmd/tink-server/main.go
{"level":"info","ts":1600772853.779405,"caller":"tink-server/main.go:30","msg":"starting version devel","service":"github.com/tinkerbell/tink"}
{"level":"info","ts":1600772853.779805,"caller":"tink-server/main.go:40","msg":"Applying migrations. This process will end when migrations will take place.","service":"github.com/tinkerbell/tink"}
{"level":"info","ts":1600772853.819854,"caller":"tink-server/main.go:45","msg":"Running migrations if necessary","service":"github.com/tinkerbell/tink","num_applied_migrations":1}
```

This is the logs from `docker-compose up`, as you can see there is a new service that runs before `tink-server` and it applies the migrations.

```
registry_1               | 127.0.0.1 - - [01/Oct/2020:08:58:07 +0000] "GET / HTTP/2.0" 200 0 "" "curl/7.67.0"
db_1                     | 2020-10-01 08:58:04.165 UTC [1] LOG:  database system is ready to accept connections
registry_1               | 127.0.0.1 - - [01/Oct/2020:08:58:12 +0000] "GET / HTTP/2.0" 200 0 "" "curl/7.67.0"
hegel_1                  | 2020/10/01 08:58:05 fetch cert: Get http://127.0.0.1:42114/cert: dial tcp 127.0.0.1:42114: connect: connection refused
tink-server-migration_1  | {"level":"info","ts":1601542686.5914426,"caller":"tink-server/main.go:29","msg":"starting version 81ae632","service":"github.com/tinkerbell/tink"}
tink-server-migration_1  | {"level":"info","ts":1601542686.591473,"caller":"tink-server/main.go:39","msg":"Applying migrations. This process will end when migrations will take place.","service":"github.com/tinkerbell/tink"}
tink-server-migration_1  | {"level":"info","ts":1601542686.6131954,"caller":"tink-server/main.go:45","msg":"Migrations applied successfully","service":"github.com/tinkerbell/tink","num_applied_migrations":0}
deploy_tink-server-migration_1 exited with code 0
tink-server_1            | {"level":"info","ts":1601542687.7541816,"caller":"tink-server/main.go:29","msg":"starting version 81ae632","service":"github.com/tinkerbell/tink"}
tink-server_1            | {"level":"info","ts":1601542687.7602122,"caller":"metrics/metrics.go:58","msg":"initializing label values","service":"github.com/tinkerbell/tink"}
tink-server_1            | {"level":"info","ts":1601542687.7629616,"caller":"grpc-server/grpc_server.go:76","msg":"serving grpc","service":"github.com/tinkerbell/tink"}
tink-server_1            | {"level":"info","ts":1601542687.7635539,"caller":"http-server/http_server.go:90","msg":"serving http","service":"github.com/tinkerbell/tink"}
```
## Why is this needed

Fixes: #295 

## How Has This Been Tested?

With `docker-compose` and `vagrant`.

## How are existing users impacted? What migration steps/scripts do we need?

no impact, the workflow is well managed by docker-compose, transparently.

They are not impacted by this change.

## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gianarb gianarb

Labels
area/setup Issue related to tinkerbell setup kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/M estimate of the amount of work to address the issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Apply migrations when tink-server boots
Apply migrations when tink-server boots
4 participants
@gianarb @thebsdbox @Cbkhare @rugwirobaker