Ack to this series.
----- Original Message -----
From: "Jason Guiditta" <jguiditt(a)redhat.com>
To: aeolus-devel(a)lists.fedorahosted.org
Sent: Monday, 28 March, 2011 10:34:04 PM
Subject: [PATCH conductor 2/2] Improve Error handling when there is no agent present.
Previously, the connector would throw a stack trace and stop
working properly if the factory agent was gone and a user
attempted an action, this patch adds logging and catches
this type of exceptionm as well as returning a 500 server error
to the caller, to be handled as desired. Future rev will return
the error string as well, but Conductor does nothing with errors
at this point anyway.
---
.../lib/image_factory/image_factory_console.rb | 20 +++++++++---
.../console/spec/image_factory/console_spec.rb | 25 ++++++++++++----
.../image_factory_connector.rb | 25 +++++++++++----
.../image_factory_connector/spec/connector_spec.rb | 32 +++++++++++++++++--
4 files changed, 80 insertions(+), 22 deletions(-)
diff --git a/services/image_factory/console/lib/image_factory/image_factory_console.rb
b/services/image_factory/console/lib/image_factory/image_factory_console.rb
index a1a5b8c..c196632 100644
--- a/services/image_factory/console/lib/image_factory/image_factory_console.rb
+++ b/services/image_factory/console/lib/image_factory/image_factory_console.rb
@@ -64,8 +64,13 @@ class ImageFactoryConsole < Qmf2::ConsoleHandler
def build_image(descriptor, target)
# TODO: return error if there is a problem calling this method or getting
# a factory instance
- response = factory.image(descriptor, target)
- build_adaptor(response)
+ begin
+ response = factory.image(descriptor, target)
+ build_adaptor(response)
+ rescue Exception => e
+ @logger.debug "Encountered error in build_image: #{e}"
+ return e
+ end
end
# Call this method to push an image to a provider, and get back an
@@ -80,8 +85,13 @@ class ImageFactoryConsole < Qmf2::ConsoleHandler
def push_image(image_id, provider, credentials)
# TODO: return error if there is a problem calling this method or getting
# a factory instance
- response = factory.provider_image(image_id, provider, credentials)
- build_adaptor(response)
+ begin
+ response = factory.provider_image(image_id, provider, credentials)
+ build_adaptor(response)
+ rescue Exception => e
+ @logger.debug "Encountered error in push_image: #{e}"
+ return e
+ end
end
#TODO: enhance both of these methods to handle multiple agents
@@ -132,7 +142,7 @@ class ImageFactoryConsole < Qmf2::ConsoleHandler
def build_adaptor(response)
imgfacaddr = Qmf2::DataAddr.new(response['build_adaptor'])
query = Qmf2::Query.new(imgfacaddr)
- @q.query(query).first
+ @q.query(query,5).first
end
end
diff --git a/services/image_factory/console/spec/image_factory/console_spec.rb
b/services/image_factory/console/spec/image_factory/console_spec.rb
index 4e3a0b1..9cd766d 100644
--- a/services/image_factory/console/spec/image_factory/console_spec.rb
+++ b/services/image_factory/console/spec/image_factory/console_spec.rb
@@ -8,7 +8,7 @@ module ImageFactory
@logger = Logger.new(STDOUT)
@logger.level = Logger::DEBUG
@logger.datetime_format = "%Y-%m-%d %H:%M:%S"
- @i = ImageFactoryConsole.new()
+ @i = ImageFactoryConsole.new() # Pass this in for more logging: (:logger =>
@logger)
@i.start
# Sadly, this is needed so we wait for an agent to appear.
# We might be able to extract this somewhere to hold off
@@ -97,11 +97,6 @@ module ImageFactory
@data.stub(:event).and_return("STATUS")
end
- it "should log an event" do
- @output.should_receive(:debug).with(/GOT AN EVENT/)
- @i2.event_raised(@agent, @data, Time.now, "horrid")
- end
-
it "should call handle on an event" do
@output.should_receive(:debug).with(/GOT AN EVENT/)
@@ -110,6 +105,24 @@ module ImageFactory
end
end
+ describe "#build_image" do
+ it "should gracefully handle errors" do
+ @output.should_receive(:debug).with(/Encountered error in build_image/)
+ @i2.q=nil
+ error = @i2.build_image("<template></template>",
"mock")
+ error.to_s.should include("undefined method")
+ end
+ end
+
+ describe "#push_image" do
+ it "should gracefully handle errors" do
+ @output.should_receive(:debug).with(/Encountered error in push_image/)
+ @i2.q=nil
+ error = @i2.push_image(123, "mock", "some creds")
+ error.to_s.should include("undefined method")
+ end
+ end
+
after(:each) do
@output.should_receive(:debug).with(/Closing/)
@i2.shutdown
diff --git
a/services/image_factory/image_factory_connector/lib/image_factory_connector/image_factory_connector.rb
b/services/image_factory/image_factory_connector/lib/image_factory_connector/image_factory_connector.rb
index 6338c09..e438dfe 100644
---
a/services/image_factory/image_factory_connector/lib/image_factory_connector/image_factory_connector.rb
+++
b/services/image_factory/image_factory_connector/lib/image_factory_connector/image_factory_connector.rb
@@ -29,11 +29,12 @@ class ImageFactoryConnector < Sinatra::Base
set :port, 2003
set :app_file, __FILE__
set :views, File.dirname(__FILE__) + '/views'
- l = Logger.new(STDOUT)
- l.level = Logger::DEBUG
- @console = ImageFactoryConsole.new({:handler=>FactoryRestHandler.new(l), :logger
=>l})
+ @l = Logger.new(STDOUT)
+ @l.level = Logger::DEBUG
+ @console = ImageFactoryConsole.new({:handler=>FactoryRestHandler.new(@l), :logger
=>@l})
@console.start
set :console, @console
+ set :logger, @l
end
# TODO: add validation for required params not being passed at all,
@@ -60,15 +61,25 @@ class ImageFactoryConnector < Sinatra::Base
end
post "/build" do
- puts "build method called with #{params.inspect}"
+ settings.logger.debug "build method called with #{params.inspect}"
@b=settings.console.build_image("#{params[:template]}",
"#{params[:target]}")
- builder :image
+ if @b.respond_to?(:image_id)
+ builder :image
+ else
+ settings.logger.error "Error Received: #{(a)b.inspect}"
+ 500
+ end
end
post "/push" do
- puts "push method called with #{params.inspect}"
+ settings.logger.debug "push method called with #{params.inspect}"
@b=settings.console.push_image("#{params[:image_id]}",
"#{params[:provider]}", "#{params[:credentials]}")
- builder :image
+ if @b.respond_to?(:image_id)
+ builder :image
+ else
+ settings.logger.error "Error Received: #{(a)b.inspect}"
+ 500
+ end
end
get "/shutdown" do
diff --git a/services/image_factory/image_factory_connector/spec/connector_spec.rb
b/services/image_factory/image_factory_connector/spec/connector_spec.rb
index 44badef..207e896 100644
--- a/services/image_factory/image_factory_connector/spec/connector_spec.rb
+++ b/services/image_factory/image_factory_connector/spec/connector_spec.rb
@@ -79,9 +79,33 @@ describe 'image_factory_connector app' do
end
end
- it 'closes the connection' do
- get 'shutdown'
- last_response.should be_ok
- last_response.body.should == "Console connection closed"
+ context "logs errors if method is called with no agent present" do
+ before(:each) do
+ app.logger = double('output')
+ app.console.q=nil
+ app.logger.should_receive(:debug).with(any_args())
+ end
+
+ it "logs a message when there is no agent present and build method is
called" do
+ app.logger.should_receive(:error).with(/Error Received/)
+ post 'build', {:template => '<template></template>',
:target => 'mock'}
+ end
+
+ it "returns a 500 error if an exception is thrown calling build in the
console" do
+ app.logger.should_receive(:error).with(/Error Received/)
+ post 'build', {:template => '<template></template>',
:target => 'mock'}
+ last_response.status.should == 500
+ end
+
+ it "logs a message when there is no agent present and push method is
called" do
+ app.logger.should_receive(:error).with(/Error Received/)
+ post 'push', {:image_id => 123, :provider => 'mock',
:credentials => 'some creds'}
+ end
+
+ it "returns a 500 error if an exception is thrown calling push in the
console" do
+ app.logger.should_receive(:error).with(/Error Received/)
+ post 'push', {:image_id => 123, :provider => 'mock',
:credentials => 'some creds'}
+ last_response.status.should == 500
+ end
end
end
--
1.7.4
_______________________________________________
aeolus-devel mailing list
aeolus-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/aeolus-devel