Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more robust error handling to ConversionHost#get_conversion_state #18258

Merged
merged 6 commits into from
Mar 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions app/models/conversion_host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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+.
Expand Down
29 changes: 29 additions & 0 deletions spec/models/conversion_host_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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