Merge pull request #18258 from djberg96/get_conversion_state

Add more robust error handling to ConversionHost#get_conversion_state

app/models/conversion_host.rb

@@ -145,17 +145,18 @@ def kill_process(pid, signal = 'TERM')
     false
   end
 
-  # Get the conversion state by reading from a remote json file at +path+
-  # and return the parsed data.
-  #--
-  # TODO: This should be more clear on failure, since it could fail because
-  # it's not found or because the contents were invalid.
+  # Retrieve the conversion state information from a remote file as a stream.
+  # Then parse and return the stream data as a hash using JSON.parse.
   #
   def get_conversion_state(path)
     json_state = connect_ssh { |ssu| ssu.get_file(path, nil) }
     JSON.parse(json_state)
-  rescue => e
-    raise "Could not get state file '#{path}' from '#{resource.name}' with [#{e.class}: #{e}"
+  rescue MiqException::MiqInvalidCredentialsError, MiqException::MiqSshUtilHostKeyMismatch => err
+    raise "Failed to connect and retrieve conversion state data from file '#{path}' with [#{err.class}: #{err}"
+  rescue JSON::ParserError
+    raise "Could not parse conversion state data from file '#{path}': #{json_state}"
+  rescue StandardError => err
+    raise "Error retrieving and parsing conversion state file '#{path}' from '#{resource.name}' with [#{err.class}: #{err}"
   end
 
   # Get and return the contents of the remote conversion log at +path+.

spec/models/conversion_host_spec.rb

@@ -309,4 +309,33 @@
       expect(conversion_host_vm.verify_credentials('v2v')).to be_truthy
     end
   end
+
+  context "#get_conversion_state" do
+    let(:vm) { FactoryBot.create(:vm_openstack) }
+    let(:conversion_host) { FactoryBot.create(:conversion_host, :resource => vm) }
+    let(:path) { 'some_path' }
+
+    it "works as expected if the connection is successful and the JSON is valid" do
+      allow(conversion_host).to receive(:connect_ssh).and_return({:alpha => {:beta => 'hello'}}.to_json)
+      expect(conversion_host.get_conversion_state(path)).to eql('alpha' => {'beta' => 'hello'})
+    end
+
+    it "works as expected if the connection is successful but the JSON is invalid" do
+      allow(conversion_host).to receive(:connect_ssh).and_return('bogus')
+      expected_message = "Could not parse conversion state data from file '#{path}': bogus"
+      expect { conversion_host.get_conversion_state(path) }.to raise_error(expected_message)
+    end
+
+    it "works as expected if the connection is unsuccessful" do
+      allow(conversion_host).to receive(:connect_ssh).and_raise(MiqException::MiqInvalidCredentialsError)
+      expected_message = "Failed to connect and retrieve conversion state data from file '#{path}'"
+      expect { conversion_host.get_conversion_state(path) }.to raise_error(/#{expected_message}/)
+    end
+
+    it "works as expected if an unknown error occurs" do
+      allow(conversion_host).to receive(:connect_ssh).and_raise(StandardError)
+      expected_message = "Error retrieving and parsing conversion state file '#{path}' from '#{vm.name}'"
+      expect { conversion_host.get_conversion_state(path) }.to raise_error(/#{expected_message}/)
+    end
+  end
 end