From 953a224d0b90f143b26f946bec5200a6b62d31c9 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Tue, 21 Aug 2018 18:56:25 -0500 Subject: [PATCH] Integrate MiqFileStorage into EvmDatabaseOps MiqFileStorage is the new wrapper class around Object Stores and mounts, will a consistent interface for interacting with both. It's job is to handle streaming command output to the source destination, without letting the contents of the commands touch the local disk (unless the target is the local disk). A call to either `.add` or `.download` is now required for the proper IO redirection to work, but the interface is consistent between all providers, and the IO is expected to be handled in the same way for each. --- lib/evm_database_ops.rb | 33 +++---- spec/lib/evm_database_ops_spec.rb | 143 ++++++++++++++++-------------- 2 files changed, 93 insertions(+), 83 deletions(-) diff --git a/lib/evm_database_ops.rb b/lib/evm_database_ops.rb index eaf61c1ed5ad..d850c51b1854 100644 --- a/lib/evm_database_ops.rb +++ b/lib/evm_database_ops.rb @@ -2,6 +2,7 @@ require 'util/postgres_admin' require 'mount/miq_generic_mount_session' +require 'util/miq_object_storage' class EvmDatabaseOps include Vmdb::Logging @@ -53,10 +54,9 @@ def self.backup(db_opts, connect_opts = {}) # :password => 'Zug-drep5s', # :remote_file_name => "backup_1", - Provide a base file name for the uploaded file - uri = with_mount_session(:backup, db_opts, connect_opts) do |database_opts, session, remote_file_uri| + uri = with_file_storage(:backup, db_opts, connect_opts) do |database_opts| validate_free_space(database_opts) backup_result = PostgresAdmin.backup(database_opts) - session&.add(database_opts[:local_file], remote_file_uri) backup_result end _log.info("[#{merged_db_opts(db_opts)[:dbname]}] database has been backed up to file: [#{uri}]") @@ -66,7 +66,7 @@ def self.backup(db_opts, connect_opts = {}) def self.dump(db_opts, connect_opts = {}) # db_opts and connect_opts similar to .backup - uri = with_mount_session(:dump, db_opts, connect_opts) do |database_opts, _session, _remote_file_uri| + uri = with_file_storage(:dump, db_opts, connect_opts) do |database_opts| # For database dumps, this isn't going to be as accurate (since the dump # size will probably be larger than the calculated BD size), but it still # won't hurt to do as a generic way to get a rough idea if we have enough @@ -89,10 +89,7 @@ def self.restore(db_opts, connect_opts = {}) # :username => 'samba_one', # :password => 'Zug-drep5s', - uri = with_mount_session(:restore, db_opts, connect_opts) do |database_opts, session, remote_file_uri| - if session && !File.exist?(database_opts[:local_file]) - database_opts[:local_file] = session.download(database_opts[:local_file], remote_file_uri) - end + uri = with_file_storage(:restore, db_opts, connect_opts) do |database_opts| prepare_for_restore(database_opts[:local_file]) # remove all the connections before we restore; AR will reconnect on the next query @@ -107,8 +104,10 @@ def self.restore(db_opts, connect_opts = {}) DEFAULT_OPTS.merge(db_opts) end - private_class_method def self.with_mount_session(action, db_opts, connect_opts) - db_opts = DEFAULT_OPTS.merge(db_opts) + STORAGE_ACTIONS_TO_METHODS = { :backup => :upload, :dump => :upload, :restore => :download }.freeze + private_class_method def self.with_file_storage(action, db_opts, connect_opts) + db_opts = merged_db_opts(db_opts) + connect_opts = connect_opts.reverse_merge(:uri => "file://") if db_opts[:local_file].nil? if action == :restore @@ -119,15 +118,19 @@ def self.restore(db_opts, connect_opts = {}) backup_folder = action == :dump ? "db_dump" : "db_backup" uri = File.join(connect_opts[:uri], backup_folder, connect_opts[:remote_file_name]) end + else + uri = db_opts[:local_file] + end - session = MiqGenericMountSession.new_session(connect_opts) - db_opts[:local_file] = session.uri_to_local_path(uri) + MiqFileStorage.with_interface_class(connect_opts) do |file_storage| + file_storage.connect + file_storage.send(STORAGE_ACTIONS_TO_METHODS[action], uri) do |input_path| + db_opts[:local_file] = input_path + yield(db_opts) + end end - block_result = yield(db_opts, session, uri) if block_given? - uri || block_result - ensure - session.disconnect if session + uri end private_class_method def self.prepare_for_restore(filename) diff --git a/spec/lib/evm_database_ops_spec.rb b/spec/lib/evm_database_ops_spec.rb index a121e1f920d9..fcbe240752a2 100644 --- a/spec/lib/evm_database_ops_spec.rb +++ b/spec/lib/evm_database_ops_spec.rb @@ -1,15 +1,23 @@ require 'util/runcmd' describe EvmDatabaseOps do + let(:file_storage) { double("MiqSmbSession", :disconnect => nil) } + let(:input_path) { "foo/bar/mkfifo" } + let(:run_db_ops) { @db_opts.dup.merge(:local_file => input_path) } + let(:tmpdir) { Rails.root.join("tmp") } + + before do + allow(MiqFileStorage).to receive(:with_interface_class).and_yield(file_storage) + end + context "#backup" do - let(:session) { double("MiqSmbSession", :disconnect => nil) } before do @connect_opts = {:username => 'blah', :password => 'blahblah', :uri => "smb://myserver.com/share"} - @db_opts = {:dbname => 'vmdb_production', :username => 'root'} - allow(MiqGenericMountSession).to receive(:new_session).and_return(session) - allow(session).to receive(:settings_mount_point).and_return(Rails.root.join("tmp").to_s) - allow(session).to receive(:uri_to_local_path).and_return(Rails.root.join("tmp/share").to_s) - allow(PostgresAdmin).to receive(:runcmd_with_logging) - allow(FileUtils).to receive(:mv).and_return(true) + @db_opts = {:username => 'root', :dbname => 'vmdb_production' } + allow(file_storage).to receive(:settings_mount_point).and_return(tmpdir.to_s) + allow(file_storage).to receive(:uri_to_local_path).and_return(tmpdir.join("share").to_s) + allow(file_storage).to receive(:add).and_yield(input_path) + + allow(FileUtils).to receive(:mv).and_return(true) allow(EvmDatabaseOps).to receive(:backup_destination_free_space).and_return(200.megabytes) allow(EvmDatabaseOps).to receive(:database_size).and_return(100.megabytes) end @@ -17,12 +25,14 @@ it "locally" do local_backup = "/tmp/backup_1" @db_opts[:local_file] = local_backup + expect(PostgresAdmin).to receive(:backup).with(run_db_ops) expect(EvmDatabaseOps.backup(@db_opts, @connect_opts)).to eq(local_backup) end it "defaults" do local_backup = "/tmp/backup_1" @db_opts[:local_file] = local_backup + expect(PostgresAdmin).to receive(:backup).with(run_db_ops) expect(EvmDatabaseOps.backup(@db_opts, {})).to eq(local_backup) end @@ -37,14 +47,14 @@ it "remotely" do @db_opts[:local_file] = nil @connect_opts[:remote_file_name] = "custom_backup" - expect(session).to receive(:add).and_return("smb://myserver.com/share/db_backup/custom_backup") + expect(PostgresAdmin).to receive(:backup).with(run_db_ops) expect(EvmDatabaseOps.backup(@db_opts, @connect_opts)).to eq("smb://myserver.com/share/db_backup/custom_backup") end it "remotely without a remote file name" do @db_opts[:local_file] = nil @connect_opts[:remote_file_name] = nil - expect(session).to receive(:add) + expect(PostgresAdmin).to receive(:backup).with(run_db_ops) expect(EvmDatabaseOps.backup(@db_opts, @connect_opts)).to match(/smb:\/\/myserver.com\/share\/db_backup\/miq_backup_.*/) end @@ -52,13 +62,14 @@ @db_opts.delete(:dbname) @db_opts[:local_file] = nil @connect_opts[:remote_file_name] = nil + run_db_ops[:dbname] = "vmdb_production" allow(described_class).to receive(:backup_file_name).and_return("miq_backup") + expect(PostgresAdmin).to receive(:backup).with(run_db_ops) log_stub = instance_double("_log") expect(described_class).to receive(:_log).twice.and_return(log_stub) expect(log_stub).to receive(:info).with(any_args) expect(log_stub).to receive(:info).with("[vmdb_production] database has been backed up to file: [smb://myserver.com/share/db_backup/miq_backup]") - expect(session).to receive(:add).and_return("smb://myserver.com/share/db_backup/miq_backup") EvmDatabaseOps.backup(@db_opts, @connect_opts) end @@ -69,10 +80,12 @@ @connect_opts = {:username => 'blah', :password => 'blahblah', :uri => "smb://myserver.com/share"} @db_opts = {:dbname => 'vmdb_production', :username => 'root'} allow(MiqSmbSession).to receive(:runcmd) - allow_any_instance_of(MiqSmbSession).to receive(:settings_mount_point).and_return(Rails.root.join("tmp")) - allow(MiqUtil).to receive(:runcmd) - allow(PostgresAdmin).to receive(:runcmd_with_logging) - allow(FileUtils).to receive(:mv).and_return(true) + allow(file_storage).to receive(:settings_mount_point).and_return(tmpdir) + allow(file_storage).to receive(:add).and_yield(input_path) + + allow(MiqUtil).to receive(:runcmd) + allow(PostgresAdmin).to receive(:runcmd_with_logging) + allow(FileUtils).to receive(:mv).and_return(true) allow(EvmDatabaseOps).to receive(:backup_destination_free_space).and_return(200.megabytes) allow(EvmDatabaseOps).to receive(:database_size).and_return(100.megabytes) end @@ -80,12 +93,14 @@ it "locally" do local_dump = "/tmp/dump_1" @db_opts[:local_file] = local_dump + expect(PostgresAdmin).to receive(:backup_pg_dump).with(run_db_ops) expect(EvmDatabaseOps.dump(@db_opts, @connect_opts)).to eq(local_dump) end it "defaults" do local_dump = "/tmp/dump_1" @db_opts[:local_file] = local_dump + expect(PostgresAdmin).to receive(:backup_pg_dump).with(run_db_ops) expect(EvmDatabaseOps.dump(@db_opts, {})).to eq(local_dump) end @@ -93,6 +108,7 @@ EvmSpecHelper.create_guid_miq_server_zone allow(EvmDatabaseOps).to receive(:backup_destination_free_space).and_return(100.megabytes) allow(EvmDatabaseOps).to receive(:database_size).and_return(200.megabytes) + expect(PostgresAdmin).to receive(:backup_pg_dump).never expect { EvmDatabaseOps.dump(@db_opts, @connect_opts) }.to raise_error(MiqException::MiqDatabaseBackupInsufficientSpace) expect(MiqQueue.where(:class_name => "MiqEvent", :method_name => "raise_evm_event").count).to eq(1) end @@ -100,12 +116,14 @@ it "remotely" do @db_opts[:local_file] = nil @connect_opts[:remote_file_name] = "custom_pg_dump" + expect(PostgresAdmin).to receive(:backup_pg_dump).with(run_db_ops) expect(EvmDatabaseOps.dump(@db_opts, @connect_opts)).to eq("smb://myserver.com/share/db_dump/custom_pg_dump") end it "remotely without a remote file name" do @db_opts[:local_file] = nil @connect_opts[:remote_file_name] = nil + expect(PostgresAdmin).to receive(:backup_pg_dump).with(run_db_ops) expect(EvmDatabaseOps.dump(@db_opts, @connect_opts)).to match(/smb:\/\/myserver.com\/share\/db_dump\/miq_pg_dump_.*/) end end @@ -114,9 +132,12 @@ before do @connect_opts = {:username => 'blah', :password => 'blahblah'} @db_opts = {:dbname => 'vmdb_production', :username => 'root'} + allow(MiqSmbSession).to receive(:runcmd) allow(MiqSmbSession).to receive(:raw_disconnect) - allow_any_instance_of(MiqSmbSession).to receive(:settings_mount_point).and_return(Rails.root.join("tmp")) + allow(file_storage).to receive(:settings_mount_point).and_return(tmpdir) + 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) @@ -151,25 +172,26 @@ end end - describe "with_mount_session (private method)" do + describe "with_file_storage (private method)" do let(:db_opts) { {} } let(:connect_opts) { {} } - let(:mount_session) { instance_double("MiqGenericMountSession") } - - before do - allow(MiqGenericMountSession).to receive(:new_session).and_return(mount_session) - end # convenience_wrapper for private method - def execute_with_mount_session(action = :backup) - described_class.send(:with_mount_session, action, db_opts, connect_opts) do |dbopts, _session, _remote_file_uri| + def execute_with_file_storage(action = :backup) + described_class.send(:with_file_storage, action, db_opts, connect_opts) do |dbopts| yield dbopts if block_given? end end - shared_examples "default with_mount_session behaviors" do + shared_examples "default with_file_storage behaviors" do + it "sets dbopts[:local_file] to the input_path" do + execute_with_file_storage do |dbopts| + expect(dbopts[:local_file]).to eq(input_path) + end + end + it "updates db_opts for the block to set the :dbname" do - execute_with_mount_session do |dbopts| + execute_with_file_storage do |dbopts| expect(dbopts[:dbname]).to eq("vmdb_production") end end @@ -178,7 +200,7 @@ def execute_with_mount_session(action = :backup) it "does not update :dbname if passed" do db_opts[:dbname] = "my_db" - execute_with_mount_session do |dbopts| + execute_with_file_storage do |dbopts| expect(dbopts[:dbname]).to eq("my_db") end end @@ -188,53 +210,40 @@ def execute_with_mount_session(action = :backup) context "with a local file" do let(:db_opts) { { :local_file => "/tmp/foo" } } - include_examples "default with_mount_session behaviors" + before { expect(file_storage).to receive(:add).and_yield(input_path) } - it "does not create a MiqGenericMountSession" do - expect(MiqGenericMountSession).to_not receive(:new_session) - execute_with_mount_session + include_examples "default with_file_storage behaviors" + + it "always uses a file_storage interface" do + execute_with_file_storage do + expect(file_storage).to receive(:test_method) + file_storage.test_method + end end it "does not try to close a session" do - expect(mount_session).to_not receive(:disconnect) + expect(file_storage).to_not receive(:disconnect) - execute_with_mount_session + execute_with_file_storage end - it "does not db_opts[:local_file] in the method context" do - execute_with_mount_session do |dbopts| - expect(dbopts[:local_file]).to eq("/tmp/foo") + it "updates the db_opts[:local_file] to the file_storage fifo" do + execute_with_file_storage do |dbopts| + expect(dbopts[:local_file]).to eq(input_path) end end it "returns the result of the block" do - expect(execute_with_mount_session { "block result" }).to eq("block result") + expect(execute_with_file_storage { "block result" }).to eq(db_opts[:local_file]) end end context "without a local file" do let(:connect_opts) { { :uri => "smb://tmp/foo" } } - include_examples "default with_mount_session behaviors" + before { allow(file_storage).to receive(:add).and_yield(input_path) } - before do - allow(mount_session).to receive(:disconnect) - # give a slightly different result for this stub so we see a difference - # in the specs. This just truncates the scheme from the passed in - # `uri` arg. - allow(mount_session).to receive(:uri_to_local_path) { |uri| uri[5..-1] } - end - - it "creates a MiqGenericMountSession" do - expect(MiqGenericMountSession).to receive(:new_session) - execute_with_mount_session - end - - it "closes the session" do - expect(mount_session).to receive(:disconnect) - - execute_with_mount_session - end + include_examples "default with_file_storage behaviors" context "for a backup-ish action" do let(:backup_file) { "/tmp/bar/baz" } @@ -242,36 +251,34 @@ def execute_with_mount_session(action = :backup) before { allow(described_class).to receive(:backup_file_name).and_return(backup_file) } it "updates db_opts[:local_file] in the method context" do - expected_filename = "/tmp/foo/db_backup/baz" + expected_uri = "smb://tmp/foo/db_backup/baz" - execute_with_mount_session do |dbopts| - expect(dbopts[:local_file]).to eq(expected_filename) - end + expect(file_storage).to receive(:send).with(:add, expected_uri) + execute_with_file_storage end it "respects user passed in connect_opts[:remote_file_name]" do - expected_filename = "/tmp/foo/db_backup/my_dir/my_backup" + expected_uri = "smb://tmp/foo/db_backup/my_dir/my_backup" connect_opts[:remote_file_name] = "/my_dir/my_backup" - execute_with_mount_session do |dbopts| - expect(dbopts[:local_file]).to eq(expected_filename) - end + expect(file_storage).to receive(:send).with(:add, expected_uri) + execute_with_file_storage end it "returns calculated uri" do - expect(execute_with_mount_session { "block result" }).to eq("smb://tmp/foo/db_backup/baz") + expect(execute_with_file_storage { "block result" }).to eq("smb://tmp/foo/db_backup/baz") end end context "for a restore action" do it "updates db_opts[:local_file] in the method context" do - execute_with_mount_session(:restore) do |dbopts| - expect(dbopts[:local_file]).to eq("/tmp/foo") - end + expect(file_storage).to receive(:send).with(:download, "smb://tmp/foo") + execute_with_file_storage(:restore) end it "returns calculated uri" do - expect(execute_with_mount_session(:restore) { "block result" }).to eq("smb://tmp/foo") + allow(file_storage).to receive(:download).and_yield(input_path) + expect(execute_with_file_storage(:restore) { "block result" }).to eq("smb://tmp/foo") end end end