Skip to content

Ansible able to migrate database on upgrade#138

Merged
mprahl merged 2 commits intomasterfrom
ft-database_migrations-125
Jul 19, 2016
Merged

Ansible able to migrate database on upgrade#138
mprahl merged 2 commits intomasterfrom
ft-database_migrations-125

Conversation

@thatarchguy
Copy link
Member

A few issues in one here:

Ansible is able to perform database migrations on upgrade - #125
We now track migrations in the repository - #129

A special command was created for users that already have the virtual_* tables in their database. (People who already installed a mailserver prior to PostMaster)
python manage.py existingdb

@thatarchguy
Copy link
Member Author

I had a merge conflict with the config.py file that turned in to a 3 way merge conflict that turned into the whole thing replaying its commits on top of itself. Sorry about that...

@@ -0,0 +1,3 @@
#!/bin/bash
pip install ansible
Copy link
Member

Choose a reason for hiding this comment

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

@thatarchguy, is there any reason you don't have the --upgrade flag on here, but you do in ansible_install.sh? Maybe we can ensure that a minimum version is installed (i.e. 2.0+)

Copy link
Member Author

Choose a reason for hiding this comment

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

I overlooked that. I have the --upgrade ansible setuptools in the install.sh script because ansible throws errors if you don't pip install setuptools after you install it from apt. It's weird I know.
And yeah we want ansible to be 2.0+ for sure.

@mprahl
Copy link
Member

mprahl commented Jul 17, 2016

@thatarchguy,
The problem I see now is that the database migrations would potentially not be handled properly in Docker based on the state of the MySQL database. When you first deploy PostMaster via Docker, you might need to run python manage.py existingdb or python manage.py createdb depending on if you followed the mail server setup guide or not.

Maybe we should just have one command that has the logic to determine what state it's in. For instance, if the alembic table does not exist and virtual_* do, you know you need to run flask_migrate.stamp(revision='bcc85aaa7896'), otherwise, you would leave that out.

@thatarchguy
Copy link
Member Author

Yeah I looked into this a lot and I couldn't figure out how to test if the tables existed or not in alembic. There were a few issues on stackoverflow and mailing lists with people in similar situations as ours. This two command thing was a compromise

@mprahl
Copy link
Member

mprahl commented Jul 17, 2016

@thatarchguy, I don't think you need to read the data in the alembic table. You just need to something like:

if not alembic_table_exists and virtual_domains_exists and virtual_users_exists and virtual_aliases_exists:
    flask_migrate.stamp(revision='bcc85aaa7896')

flask_migrate.upgrade()
add_default_configuration_settings()

Edit:
You can check if the table exists with this code:

db.engine.dialect.has_table(db.session.connection(), 'virtual_users')

Ansible now migrates database on upgrade

Signed-off-by: Kevin <kevin@stealsyour.pw>
@mprahl mprahl force-pushed the ft-database_migrations-125 branch from d1fdd5c to 008215d Compare July 18, 2016 00:57
Sets the minimum version of Ansible to use at 2.0.0.1 so it doesn't automatically upgrade a system's Ansible if it isn't necessary
Formatting fixes
@mprahl mprahl force-pushed the ft-database_migrations-125 branch from 008215d to 725d5ca Compare July 19, 2016 02:07
@mprahl
Copy link
Member

mprahl commented Jul 19, 2016

@thatarchguy really nice job on this. I think my last commit addresses our issues. I'm ready to merge this if you are.

@thatarchguy
Copy link
Member Author

Lgtm! Merge at will

@thatarchguy thatarchguy assigned mprahl and unassigned thatarchguy Jul 19, 2016
@mprahl
Copy link
Member

mprahl commented Jul 19, 2016

LGTM

@mprahl mprahl merged commit 541a412 into master Jul 19, 2016
@mprahl mprahl deleted the ft-database_migrations-125 branch July 19, 2016 23:47
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.

2 participants