FEATURE: Post deployment migrations. (#6406)

This moves us away from the delayed drops pattern which
was problematic on two counts. First, it uses a hardcoded "delay for"
duration which may be too short for certain deployment strategies.
Second, delayed drop doesn't ensure that it only runs after
the latest application code has been deployed. If the migration runs
and the application code fails to deploy, running the migration after
"delay for" has been met will cause the application to blow up.

The new strategy allows post deployment migrations to be skipped if the
env `SKIP_POST_DEPLOYMENT_MIGRATIONS` is provided.

```
SKIP_POST_DEPLOYMENT_MIGRATIONS=1 rake db:migrate
-> deploy app servers
SKIP_POST_DEPLOYMENT_MIGRATIONS=0 rake db:migrate
```

To aid with the generation of a post deployment migration, a generator
has been added. Simply run `rails generate post_migration`.
This commit is contained in:
Guo Xiang Tan
2018-10-08 15:47:38 +08:00
committed by GitHub
parent 9bbc1ae7b2
commit 40fa96777d
26 changed files with 260 additions and 738 deletions

View File

@@ -2,7 +2,6 @@ require 'rails_helper'
require_dependency 'migration/column_dropper'
RSpec.describe Migration::ColumnDropper do
def has_column?(table, column)
DB.exec(<<~SQL, table: table, column: column) == 1
SELECT 1
@@ -14,105 +13,27 @@ RSpec.describe Migration::ColumnDropper do
SQL
end
def update_first_migration_date(created_at)
DB.exec(<<~SQL, created_at: created_at)
UPDATE schema_migration_details
SET created_at = :created_at
WHERE id = (SELECT MIN(id)
FROM schema_migration_details)
SQL
end
describe ".drop" do
let(:migration_name) do
DB.query_single("SELECT name FROM schema_migration_details ORDER BY id DESC LIMIT 1").first
end
describe ".execute_drop" do
let(:columns) { %w{junk junk2} }
before do
DB.exec "ALTER TABLE topics ADD COLUMN junk int"
DB.exec(<<~SQL, name: migration_name, created_at: 15.minutes.ago)
UPDATE schema_migration_details
SET created_at = :created_at
WHERE name = :name
SQL
columns.each do |column|
DB.exec("ALTER TABLE topics ADD COLUMN #{column} int")
end
end
it "can correctly drop columns after correct delay" do
dropped_proc_called = false
after_dropped_proc_called = false
update_first_migration_date(2.years.ago)
Migration::ColumnDropper.drop(
table: 'topics',
after_migration: migration_name,
columns: ['junk'],
delay: 20.minutes,
on_drop: ->() { dropped_proc_called = true },
after_drop: ->() { after_dropped_proc_called = true }
)
expect(has_column?('topics', 'junk')).to eq(true)
expect(dropped_proc_called).to eq(false)
expect(dropped_proc_called).to eq(false)
Migration::ColumnDropper.drop(
table: 'topics',
after_migration: migration_name,
columns: ['junk'],
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true },
after_drop: ->() { after_dropped_proc_called = true }
)
expect(has_column?('topics', 'junk')).to eq(false)
expect(dropped_proc_called).to eq(true)
expect(after_dropped_proc_called).to eq(true)
dropped_proc_called = false
after_dropped_proc_called = false
Migration::ColumnDropper.drop(
table: 'topics',
after_migration: migration_name,
columns: ['junk'],
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true },
after_drop: ->() { after_dropped_proc_called = true }
)
# it should call "on_drop" only when there are columns to drop
expect(dropped_proc_called).to eq(false)
expect(after_dropped_proc_called).to eq(false)
after do
columns.each do |column|
DB.exec("ALTER TABLE topics DROP COLUMN IF EXISTS #{column}")
end
end
it "drops the columns immediately if the first migration was less than 10 minutes ago" do
dropped_proc_called = false
update_first_migration_date(11.minutes.ago)
it "drops the columns" do
Migration::ColumnDropper.execute_drop("topics", columns)
Migration::ColumnDropper.drop(
table: 'topics',
after_migration: migration_name,
columns: ['junk'],
delay: 30.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(has_column?('topics', 'junk')).to eq(true)
expect(dropped_proc_called).to eq(false)
update_first_migration_date(9.minutes.ago)
Migration::ColumnDropper.drop(
table: 'topics',
after_migration: migration_name,
columns: ['junk'],
delay: 30.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(has_column?('topics', 'junk')).to eq(false)
expect(dropped_proc_called).to eq(true)
columns.each do |column|
expect(has_column?('topics', column)).to eq(false)
end
end
end
@@ -135,23 +56,18 @@ RSpec.describe Migration::ColumnDropper do
DB.exec <<~SQL
DROP TABLE IF EXISTS #{table_name};
DROP TRIGGER IF EXISTS #{table_name}_email_readonly ON #{table_name};
DROP FUNCTION IF EXISTS #{Migration::BaseDropper.readonly_function_name(table_name, 'email')} CASCADE;
SQL
end
it 'should be droppable' do
name = DB.query_single("SELECT name FROM schema_migration_details LIMIT 1").first
Migration::ColumnDropper.execute_drop(table_name, ['email'])
dropped_proc_called = false
Migration::ColumnDropper.drop(
table: table_name,
after_migration: name,
columns: ['email'],
delay: 0.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(has_trigger?(Migration::BaseDropper.readonly_trigger_name(
table_name, 'email'
))).to eq(false)
expect(dropped_proc_called).to eq(true)
expect(has_column?(table_name, 'email')).to eq(false)
end
it 'should prevent updates to the readonly column' do

View File

@@ -29,7 +29,7 @@ describe Migration::SafeMigrate do
it "bans all table removal" do
Migration::SafeMigrate.enable!
path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/drop_table"
path = File.expand_path "#{Rails.root}/spec/fixtures/db/migrate/drop_table"
output = capture_stdout do
expect(lambda do
@@ -37,7 +37,7 @@ describe Migration::SafeMigrate do
end).to raise_error(StandardError)
end
expect(output).to include("TableDropper")
expect(output).to include("rails g post_migration")
expect { User.first }.not_to raise_error
expect(User.first).not_to eq(nil)
@@ -46,7 +46,7 @@ describe Migration::SafeMigrate do
it "bans all table renames" do
Migration::SafeMigrate.enable!
path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/rename_table"
path = File.expand_path "#{Rails.root}/spec/fixtures/db/migrate/rename_table"
output = capture_stdout do
expect(lambda do
@@ -57,13 +57,13 @@ describe Migration::SafeMigrate do
expect { User.first }.not_to raise_error
expect(User.first).not_to eq(nil)
expect(output).to include("TableDropper")
expect(output).to include("rails g post_migration")
end
it "bans all column removal" do
Migration::SafeMigrate.enable!
path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/remove_column"
path = File.expand_path "#{Rails.root}/spec/fixtures/db/migrate/remove_column"
output = capture_stdout do
expect(lambda do
@@ -71,7 +71,7 @@ describe Migration::SafeMigrate do
end).to raise_error(StandardError)
end
expect(output).to include("ColumnDropper")
expect(output).to include("rails g post_migration")
expect(User.first).not_to eq(nil)
expect { User.first.username }.not_to raise_error
@@ -80,7 +80,7 @@ describe Migration::SafeMigrate do
it "bans all column renames" do
Migration::SafeMigrate.enable!
path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/rename_column"
path = File.expand_path "#{Rails.root}/spec/fixtures/db/migrate/rename_column"
output = capture_stdout do
expect(lambda do
@@ -88,7 +88,7 @@ describe Migration::SafeMigrate do
end).to raise_error(StandardError)
end
expect(output).to include("ColumnDropper")
expect(output).to include("rails g post_migration")
expect(User.first).not_to eq(nil)
expect { User.first.username }.not_to raise_error
@@ -98,7 +98,7 @@ describe Migration::SafeMigrate do
Migration::SafeMigrate.enable!
Migration::SafeMigrate.disable!
path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/drop_table"
path = File.expand_path "#{Rails.root}/spec/fixtures/db/migrate/drop_table"
output = capture_stdout do
migrate_up(path)
@@ -106,4 +106,20 @@ describe Migration::SafeMigrate do
expect(output).to include("drop_table(:users)")
end
describe 'for a post deployment migration' do
it 'should not ban unsafe migrations' do
user = Fabricate(:user)
Migration::SafeMigrate::SafeMigration.enable_safe!
path = File.expand_path "#{Rails.root}/spec/fixtures/db/post_migrate/drop_table"
output = capture_stdout do
migrate_up(path)
end
expect(output).to include("drop_table(:users)")
expect(user.reload).to eq(user)
end
end
end

View File

@@ -4,200 +4,68 @@ require_dependency 'migration/table_dropper'
describe Migration::TableDropper do
def table_exists?(table_name)
sql = <<~SQL
DB.exec(<<~SQL) > 0
SELECT 1
FROM INFORMATION_SCHEMA.TABLES
WHERE table_schema = 'public' AND
table_name = '#{table_name}'
SQL
DB.exec(sql) > 0
end
def update_first_migration_date(created_at)
DB.exec(<<~SQL, created_at: created_at)
UPDATE schema_migration_details
SET created_at = :created_at
WHERE id = (SELECT MIN(id)
FROM schema_migration_details)
SQL
end
def create_new_table
DB.exec "CREATE TABLE table_with_new_name (topic_id INTEGER)"
end
let(:migration_name) do
DB.query_single("SELECT name FROM schema_migration_details ORDER BY id DESC LIMIT 1").first
end
let(:table_name) { 'table_with_old_name' }
before do
DB.exec "CREATE TABLE table_with_old_name (topic_id INTEGER)"
DB.exec "CREATE TABLE #{table_name} (topic_id INTEGER)"
DB.exec(<<~SQL, name: migration_name, created_at: 15.minutes.ago)
UPDATE schema_migration_details
SET created_at = :created_at
WHERE name = :name
DB.exec <<~SQL
INSERT INTO #{table_name} (topic_id) VALUES (1)
SQL
end
context "first migration was a long time ago" do
before do
update_first_migration_date(2.years.ago)
end
describe ".execute_drop" do
it "should drop the table" do
Migration::TableDropper.execute_drop(table_name)
describe ".delayed_rename" do
it "can drop a table after correct delay and when new table exists" do
dropped_proc_called = false
described_class.delayed_rename(
old_name: 'table_with_old_name',
new_name: 'table_with_new_name',
after_migration: migration_name,
delay: 20.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(true)
expect(dropped_proc_called).to eq(false)
described_class.delayed_rename(
old_name: 'table_with_old_name',
new_name: 'table_with_new_name',
after_migration: migration_name,
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(true)
expect(dropped_proc_called).to eq(false)
create_new_table
described_class.delayed_rename(
old_name: 'table_with_old_name',
new_name: 'table_with_new_name',
after_migration: migration_name,
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(false)
expect(dropped_proc_called).to eq(true)
dropped_proc_called = false
described_class.delayed_rename(
old_name: 'table_with_old_name',
new_name: 'table_with_new_name',
after_migration: migration_name,
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true }
)
# it should call "on_drop" only when there is a table to drop
expect(dropped_proc_called).to eq(false)
end
end
describe ".delayed_drop" do
it "can drop a table after correct delay" do
dropped_proc_called = false
described_class.delayed_drop(
table_name: 'table_with_old_name',
after_migration: migration_name,
delay: 20.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(true)
expect(dropped_proc_called).to eq(false)
described_class.delayed_drop(
table_name: 'table_with_old_name',
after_migration: migration_name,
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(false)
expect(dropped_proc_called).to eq(true)
dropped_proc_called = false
described_class.delayed_drop(
table_name: 'table_with_old_name',
after_migration: migration_name,
delay: 10.minutes,
on_drop: ->() { dropped_proc_called = true }
)
# it should call "on_drop" only when there is a table to drop
expect(dropped_proc_called).to eq(false)
end
expect(table_exists?(table_name)).to eq(false)
end
end
context "first migration was a less than 10 minutes ago" do
describe ".delayed_rename" do
it "can drop a table after correct delay and when new table exists" do
dropped_proc_called = false
update_first_migration_date(11.minutes.ago)
create_new_table
described_class.delayed_rename(
old_name: 'table_with_old_name',
new_name: 'table_with_new_name',
after_migration: migration_name,
delay: 30.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(true)
expect(dropped_proc_called).to eq(false)
update_first_migration_date(9.minutes.ago)
described_class.delayed_rename(
old_name: 'table_with_old_name',
new_name: 'table_with_new_name',
after_migration: migration_name,
delay: 30.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?('table_with_old_name')).to eq(false)
expect(dropped_proc_called).to eq(true)
end
describe ".readonly_only_table" do
before do
Migration::TableDropper.read_only_table(table_name)
end
describe ".delayed_drop" do
it "immediately drops the table" do
dropped_proc_called = false
update_first_migration_date(11.minutes.ago)
after do
ActiveRecord::Base.connection.reset!
described_class.delayed_drop(
table_name: 'table_with_old_name',
after_migration: migration_name,
delay: 30.minutes,
on_drop: ->() { dropped_proc_called = true }
)
DB.exec(<<~SQL)
DROP TABLE IF EXISTS #{table_name};
DROP FUNCTION IF EXISTS #{Migration::BaseDropper.readonly_function_name(table_name)} CASCADE;
SQL
end
expect(table_exists?('table_with_old_name')).to eq(true)
expect(dropped_proc_called).to eq(false)
it 'should be droppable' do
Migration::TableDropper.execute_drop(table_name)
update_first_migration_date(9.minutes.ago)
expect(has_trigger?(Migration::BaseDropper.readonly_trigger_name(
table_name
))).to eq(false)
described_class.delayed_drop(
table_name: 'table_with_old_name',
after_migration: migration_name,
delay: 30.minutes,
on_drop: ->() { dropped_proc_called = true }
)
expect(table_exists?(table_name)).to eq(false)
end
expect(table_exists?('table_with_old_name')).to eq(false)
expect(dropped_proc_called).to eq(true)
it 'should prevent insertions to the table' do
begin
DB.exec <<~SQL
INSERT INTO #{table_name} (topic_id) VALUES (2)
SQL
rescue PG::RaiseException => e
[
"Discourse: #{table_name} is read only",
'discourse_functions.raise_table_with_old_name_readonly()'
].each do |message|
expect(e.message).to include(message)
end
end
end
end