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

Migration tool #117

Closed
wants to merge 10 commits into from
Closed

Conversation

rolandshoemaker
Copy link
Contributor

This adds a basic migration tool (#111) boulder-migrator (in boulder/cmd/boulder-migrator/main.go) that can be used to apply migrations, revert the last migration, revert to a specific migration, and list currently applied migrations.

The tool uses a SQL table, migrations to store information about migrations that have already been applied that looks like this

id          INTEGER  # A unique identifier 
description TEXT     # A short desc of what the migration does
applied     DATETIME # When the migration was applied
down        TEXT     # The SQL statements required to revert this migration

By packaging migrations into a JSON file containing both the migration SQL and the reversion SQL we can easily rollback the last migration, or to an arbitary migration ID, by storing the reversion SQL in the migrations table. A basic migration file might look something like this

{
    "id": 1,
    "desc": "set type of base64 fields to CHAR(43)",
    "upSql": [
        "ALTER TABLE registrations MODIFY COLUMN id CHAR(43)",
        "ALTER TABLE registrations MODIFY COLUMN thumbprint CHAR(43)",
        "ALTER TABLE pending_authz MODIFY COLUMN id CHAR(43)",
        "ALTER TABLE authz MODIFY COLUMN id CHAR(43)",
        "ALTER TABLE authz MODIFY COLUMN digest CHAR(43)",
        "ALTER TABLE certificates MODIFY COLUMN digest CHAR(43)"
    ],
    "downSql": [
        "ALTER TABLE registrations MODIFY COLUMN id TEXT",
        "ALTER TABLE registrations MODIFY COLUMN thumbprint TEXT",
        "ALTER TABLE pending_authz MODIFY COLUMN id TEXT",
        "ALTER TABLE authz MODIFY COLUMN id TEXT",
        "ALTER TABLE authz MODIFY COLUMN digest TEXT",
        "ALTER TABLE certificates MODIFY COLUMN digest TEXT"
    ]
}

This file can be applied using the subcommand apply

$ ./boulder-migrator apply test.json
[migration 1]
    Description: set type of base64 fields to CHAR(43)

    Migrate SQL:
    ALTER TABLE registrations MODIFY COLUMN id CHAR(43)
    ALTER TABLE registrations MODIFY COLUMN thumbprint CHAR(43)
    ALTER TABLE pending_authz MODIFY COLUMN id CHAR(43)
    ALTER TABLE authz MODIFY COLUMN id CHAR(43)
    ALTER TABLE authz MODIFY COLUMN digest CHAR(43)
    ALTER TABLE certificates MODIFY COLUMN digest CHAR(43)

    Revert SQL:
    ALTER TABLE registrations MODIFY COLUMN id TEXT
    ALTER TABLE registrations MODIFY COLUMN thumbprint TEXT
    ALTER TABLE pending_authz MODIFY COLUMN id TEXT
    ALTER TABLE authz MODIFY COLUMN id TEXT
    ALTER TABLE authz MODIFY COLUMN digest TEXT
    ALTER TABLE certificates MODIFY COLUMN digest TEXT


Would you like to apply this migration? [y/n]
# y
Migration 1 applied

The applied migrations can be listed using list-applied

$ ./boulder-migrator list-applied
# Currently applied migrations

1: [2015-04-25 03:35:23 +0000 UTC]
    set type of base64 fields to CHAR(43)

and the last migration can be reverted using revert-last

$ ./boulder-migrator revert-last
Migration 1 reverted

etc etc etc.

The help dialog should pretty much explain what is going on...

$ ./boulder-migrator help
NAME:
   boulder-migrator - A new cli application

USAGE:
   boulder-migrator [global options] command [command options] [arguments...]

VERSION:
   0.0.1

AUTHOR:
  Author - <unknown@email>

COMMANDS:
   list-applied List all currently applied migrations
   apply    Apply a migration from a JSON migration file
   revert-last  Revert the last migration that was applied
   revert-to    Revert to a specific migration specified by its ID
   help, h  Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --config "config.json"    [$BOULDER_CONFIG]
   --yes            automatically say yes to all prompts
   --help, -h           show help
   --version, -v        print the version

Side note

One note, which may actually apply to other parts of the SA code base, to get DATETIME columns to Scan to a time.Time object when using MySQL the DSN parseTime=true needs to be present in the connection URI or Scan will throw the error Scan error on column index 2: unsupported driver -> Scan pair: []uint8 -> *time.Time.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 33.61% when pulling d265d91 on rolandshoemaker:migrator into 3814c95 on letsencrypt:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.25%) to 33.36% when pulling d265d91 on rolandshoemaker:migrator into 3814c95 on letsencrypt:master.

@jsha
Copy link
Contributor

jsha commented Apr 27, 2015

Looks good so far based on the description. One high-level comment: The most common thing we're likely to want is to apply all unapplied migrations, and we will want to apply them in order. I think what most ORMs do is to name files with a time/datestamp at the front, and treat that as the explicit order and id, rather than embedded the id in the migration file itself. Also we'll want a command that applies all unapplied migrations.

return
}
}
for _, sqlStmt := range migration.DownSQL {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this for loop doesn't need to be here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, didn't see that the output of this loop is used in the next statement. But see comment below.

@jsha
Copy link
Contributor

jsha commented Apr 28, 2015

Done reviewing. Generally very excited about this, thanks for implementing so fast!

…n dir, added various util functions for future work on apply-unapplied subcommand and such
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.25%) to 33.36% when pulling 29fb650 on rolandshoemaker:migrator into 3814c95 on letsencrypt:master.

@jsha
Copy link
Contributor

jsha commented Apr 28, 2015

Is this ready for re-review? Also, I'm very confused about why Travis is failing. Have you merged the latest master?

@rolandshoemaker
Copy link
Contributor Author

It should be now!

I've added a migration directory flag/envvar (--migration-dir or BOULDER_MIGRATION_DIR) used to specify where the tool should look for the migration files. I've also implemented the apply-all subcommand which will apply all the currently unapplied migrations it finds in the migration directory. list will now list both applied and unapplied migrations and mark this state so it is easier to see which migrations in the migration directory are yet to be applied.

@rolandshoemaker
Copy link
Contributor Author

Also very confused about why Travis is failing... (merged latest master before pushing my most recent commits)

@coveralls
Copy link

Coverage Status

Coverage increased (+5.19%) to 38.55% when pulling b95a374 on rolandshoemaker:migrator into 6fe1a8b on letsencrypt:master.

@jcjones
Copy link
Contributor

jcjones commented May 1, 2015

I've retriggered the Travis build and will try it locally after seeing what happens @ Travis.

@rolandshoemaker
Copy link
Contributor Author

Seems a %s where a %d was supposed to be was breaking the build.

@coveralls
Copy link

Coverage Status

Coverage increased (+5.19%) to 38.55% when pulling f0b013d on rolandshoemaker:migrator into 6fe1a8b on letsencrypt:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+13.59%) to 46.95% when pulling d796c80 on rolandshoemaker:migrator into 6fe1a8b on letsencrypt:master.

@bdaehlie
Copy link

Been a while since there was any activity here, are we still planning to take this?

@jcjones
Copy link
Contributor

jcjones commented May 31, 2015

@bdaehlie I don't want to diminish roland any, but I would rather just use db-migrate or one of the other well-supported migration tools than to roll our own.

@bdaehlie
Copy link

If we're going to use an existing db migration tool, then can we close issue #111, right? Are we confident that another tool will work for us?

@rolandshoemaker
Copy link
Contributor Author

I'd be happy to scrap this, I think db-migrate, or, if we wanted to stick
with Go, goose (https://bitbucket.org/liamstask/goose) would work perfectly
well for this problem!
On Sun, May 31, 2015 at 5:09 PM Josh Aas notifications@github.com wrote:

If we're going to use an existing db migration tool, then can we close
issue #111 #111, right?
Are we confident that another tool will work for us?


Reply to this email directly or view it on GitHub
#117 (comment).

@bdaehlie
Copy link

I recommend we close this pull request, change issue #111 to "question" rather than "enhancement", then close it out after selecting an existing tool to use.

@bdaehlie bdaehlie closed this May 31, 2015
@jcjones
Copy link
Contributor

jcjones commented May 31, 2015

That sounds great. Thanks guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants