Skip to content

Commit

Permalink
Add support for magic check in EvmDatabaseOps
Browse files Browse the repository at this point in the history
Calls the newly created method `MiqFileStorage.magic_number_for` to
pre-fetch the magic from the file that will be downloaded, and send that
to `PostgresAdmin` as a `database_opts` to avoid checking it on it's
end.

Note:  Shortened a comment in here, but since it is part of the git
history, I am fine with it.  Those willing to dig deep can find it.
  • Loading branch information
NickLaMuro committed Nov 12, 2018
1 parent f489784 commit ca92710
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 14 deletions.
32 changes: 20 additions & 12 deletions lib/evm_database_ops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ def self.restore(db_opts, connect_opts = {})
# :username => 'samba_one',
# :password => 'Zug-drep5s',

uri = with_file_storage(:restore, db_opts, connect_opts) do |database_opts|
prepare_for_restore(database_opts[:local_file])
uri = with_file_storage(:restore, db_opts, connect_opts) do |database_opts, backup_type|
prepare_for_restore(database_opts[:local_file], backup_type)

# remove all the connections before we restore; AR will reconnect on the next query
ActiveRecord::Base.connection_pool.disconnect!
PostgresAdmin.restore(database_opts)
PostgresAdmin.restore(database_opts.merge(:backup_type => backup_type))
end
_log.info("[#{merged_db_opts(db_opts)[:dbname]}] database has been restored from file: [#{uri}]")
uri
Expand Down Expand Up @@ -139,12 +139,16 @@ def self.restore(db_opts, connect_opts = {})
MiqFileStorage.with_interface_class(connect_opts) do |file_storage|
send_args = [uri, db_opts[:byte_count]].compact

# Okay, this probably seems a bit silly seeing that we just went to the
# trouble of doing a `.compact)`. The intent is that with a restore, we
# are doing a `MiqFileStorage.download`, and the interface for that
# method is to pass a `nil` for the block form since we are streaming the
# data from the command that we are writting as part of the block.
send_args.unshift(nil) if action == :restore
if action == :restore
# `MiqFileStorage#download` requires a `nil` passed to the block form
# to accommodate streaming
send_args.unshift(nil)
magic_numbers = {
:pgdump => PostgresAdmin::PG_DUMP_MAGIC,
:basebackup => PostgresAdmin::BASE_BACKUP_MAGIC
}
backup_type = file_storage.magic_number_for(uri, :accepted => magic_numbers)
end

# Note: `input_path` will always be a fifo stream (input coming from
# PostgresAdmin, and the output going to the `uri`), since we want to
Expand All @@ -161,15 +165,19 @@ def self.restore(db_opts, connect_opts = {})
# set `db_opts` local file to that stream.
file_storage.send(STORAGE_ACTIONS_TO_METHODS[action], *send_args) do |input_path|
db_opts[:local_file] = input_path
yield(db_opts)
if action == :restore
yield(db_opts, backup_type)
else
yield(db_opts)
end
end
end

uri
end

private_class_method def self.prepare_for_restore(filename)
backup_type = validate_backup_file_type(filename)
private_class_method def self.prepare_for_restore(filename, backup_type = nil)
backup_type ||= validate_backup_file_type(filename)

if application_connections?
message = "Database restore failed. Shut down all evmserverd processes before attempting a database restore"
Expand Down
23 changes: 21 additions & 2 deletions spec/lib/evm_database_ops_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,10 @@
allow(MiqSmbSession).to receive(:runcmd)
allow(MiqSmbSession).to receive(:raw_disconnect)
allow(file_storage).to receive(:settings_mount_point).and_return(tmpdir)
allow(file_storage).to receive(:magic_number_for).and_return(:pgdump)
allow(file_storage).to receive(:download).and_yield(input_path)

allow(PostgresAdmin).to receive(:runcmd_with_logging)
allow(PostgresAdmin).to receive(:pg_dump_file?).and_return(true)
allow(PostgresAdmin).to receive(:base_backup_file?).and_return(false)

allow(VmdbDatabaseConnection).to receive(:count).and_return(1)
end
Expand Down Expand Up @@ -283,9 +282,18 @@ def execute_with_file_storage(action = :backup)
it "returns calculated uri" do
expect(execute_with_file_storage { "block result" }).to eq("smb://tmp/foo/db_backup/baz")
end

it "yields `db_opt`s only" do
allow(file_storage).to receive(:download) { |&block| block.call(input_path) }
expect do |rspec_probe|
described_class.send(:with_file_storage, :backup, db_opts, connect_opts, &rspec_probe)
end.to yield_with_args(:dbname => "vmdb_production", :local_file => input_path)
end
end

context "for a restore action" do
before { expect(file_storage).to receive(:magic_number_for).and_return(:pgdump) }

it "updates db_opts[:local_file] in the method context" do
expect(file_storage).to receive(:send).with(:download, nil, "smb://tmp/foo")
execute_with_file_storage(:restore)
Expand All @@ -303,6 +311,17 @@ def execute_with_file_storage(action = :backup)
expect(execute_with_file_storage(:restore) { "block result" }).to eq("smb://tmp/foo")
end

it "yields `backup_type` along with `db_opt`s" do
allow(file_storage).to receive(:download) { |&block| block.call(input_path) }
expected_yield_args = [
{ :dbname => "vmdb_production", :local_file => input_path },
:pgdump
]
expect do |rspec_probe|
described_class.send(:with_file_storage, :restore, db_opts, connect_opts, &rspec_probe)
end.to yield_with_args(*expected_yield_args)
end

context "with query_params in the URI" do
let(:connect_opts) { { :uri => "swift://container/foo.gz?2plus2=5" } }

Expand Down

0 comments on commit ca92710

Please sign in to comment.