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

@@ -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)

spec/lib/evm_database_ops_spec.rb

@@ -1,28 +1,38 @@
 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
 
     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,28 +47,29 @@
     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
 
     it "properly logs the result with no :dbname provided" do
       @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,43 +80,50 @@
       @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
 
     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
 
     it "without enough free space" do
       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
 
     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,90 +210,75 @@ 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" }
 
         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