These patches started with me tryingto fix whatever was going wrong in the Rackspace driver. In the process I improved the infrastructure for reporting errors from talking to a backend cloud into the XML and HTML responses.
David
From: David Lutterkort lutter@redhat.com
Commit f9dfecfe did not set API_DRIVER, API_HOST or API_PORT, so that we were always running the mock driver on localhost:3001 --- server/bin/deltacloudd | 29 ++++++++++++++++++----------- 1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/server/bin/deltacloudd b/server/bin/deltacloudd index eb51a99..0d0bce0 100755 --- a/server/bin/deltacloudd +++ b/server/bin/deltacloudd @@ -5,9 +5,7 @@ require 'optparse' require 'thin'
options = { - :env => 'development', - :host => 'localhost', - :port => '3001' + :env => 'development' } optparse = OptionParser.new do |opts|
@@ -17,9 +15,16 @@ deltacloudd -i <driver> [options]
Options: BANNER - opts.on( '-i', '--driver DRIVER', 'Driver to use') { |driver| options[:driver] = driver } - opts.on( '-r', '--hostname HOSTNAME', 'Bind to HOST address (default: localhost)') { |host| options[:host] = host } - opts.on( '-p', '--port PORT', 'Use PORT (default: 3000)') { |port| options[:port] = port } + opts.on( '-i', '--driver DRIVER', 'Driver to use') do |driver| + ENV["API_DRIVER"] = driver + end + opts.on( '-r', '--hostname HOSTNAME', + 'Bind to HOST address (default: localhost)') do |host| + ENV["API_HOST"] = host + end + opts.on( '-p', '--port PORT', 'Use PORT (default: 3001)') do |port| + ENV["API_PORT"] = port + end opts.on( '-e', '--env ENV', 'Environment (default: "development")') { |env| options[:env] = env } opts.on( '-h', '--help', '') { options[:help] = true } end @@ -31,17 +36,20 @@ if options[:help] exit(0) end
-unless options[:driver] +unless ENV["API_DRIVER"] puts "You need to specify a driver to use (-i <driver>)" exit(1) end
+ENV["API_HOST"] = "localhost" unless ENV["API_HOST"] +ENV["API_PORT"] = "3001" unless ENV["API_PORT"] + dirname="#{File.dirname(__FILE__)}/.."
argv_opts = ARGV.clone argv_opts << ['start'] unless Thin::Runner.commands.include?(options[0]) -argv_opts << ['--address', options[:host] ] -argv_opts << ['--port', options[:port] ] +argv_opts << ['--address', ENV["API_HOST"] ] +argv_opts << ['--port', ENV["API_PORT"] ] argv_opts << ['--rackup', 'config.ru' ] argv_opts << ['--chdir', dirname ] argv_opts << ['-e', options[:env] ] @@ -52,10 +60,9 @@ argv_opts.flatten! thin = Thin::Runner.new(argv_opts)
begin - puts "Starting Deltacloud API :: #{options[:driver]} :: http://#%7Boptions%5B:host%5D%7D:#%7Boptions%5B:port%5D%7D/api" + puts "Starting Deltacloud API :: #{ENV["API_DRIVER"]} :: http://#%7BENV%5B%22API_HOST%22%5D%7D:#%7BENV%5B%22API_PORT%22%5D%7D/api" puts thin.run! rescue Exception => e puts "ERROR: #{e.message}" end -
From: David Lutterkort lutter@redhat.com
* add pretty HTML output * error.xml.haml into a errors/ subdirectory * add a status attribute to the XML error --- server/server.rb | 3 ++- server/views/error.xml.haml | 7 ------- server/views/errors/validation_failure.html.haml | 11 +++++++++++ server/views/errors/validation_failure.xml.haml | 7 +++++++ 4 files changed, 20 insertions(+), 8 deletions(-) delete mode 100644 server/views/error.xml.haml create mode 100644 server/views/errors/validation_failure.html.haml create mode 100644 server/views/errors/validation_failure.xml.haml
diff --git a/server/server.rb b/server/server.rb index 535847a..6a0f4cc 100644 --- a/server/server.rb +++ b/server/server.rb @@ -73,7 +73,8 @@ error Deltacloud::Validation::Failure do $stdout.flush response.status = 400 respond_to do |format| - format.xml { haml :error, :layout => false } + format.xml { haml :"errors/validation_failure", :layout => false } + format.html { haml :"errors/validation_failure" } end end
diff --git a/server/views/error.xml.haml b/server/views/error.xml.haml deleted file mode 100644 index d7ef1af..0000000 --- a/server/views/error.xml.haml +++ /dev/null @@ -1,7 +0,0 @@ -%error{:url => "#{request.env['REQUEST_URI']}"} - %parameter #{@error.name} - %message #{@error.message} - - unless @error.param.options.empty? - %valid_options - - @error.param.options.each do |v| - %value #{v} diff --git a/server/views/errors/validation_failure.html.haml b/server/views/errors/validation_failure.html.haml new file mode 100644 index 0000000..1d3ee48 --- /dev/null +++ b/server/views/errors/validation_failure.html.haml @@ -0,0 +1,11 @@ +%h1 Validation Failure + +%p= @error.message + +%dl + %di + %dt Request URL + %dd= request.env['REQUEST_URI'] + %di + %dt Parameter + %dd= @error.name diff --git a/server/views/errors/validation_failure.xml.haml b/server/views/errors/validation_failure.xml.haml new file mode 100644 index 0000000..24519ed --- /dev/null +++ b/server/views/errors/validation_failure.xml.haml @@ -0,0 +1,7 @@ +%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"} + %parameter #{@error.name} + %message #{@error.message} + - unless @error.param.options.empty? + %valid_options + - @error.param.options.each do |v| + %value #{v}
From: David Lutterkort lutter@redhat.com
--- server/server.rb | 17 ++++++++++++----- server/views/errors/auth_exception.html.haml | 8 ++++++++ server/views/errors/auth_exception.xml.haml | 2 ++ 3 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 server/views/errors/auth_exception.html.haml create mode 100644 server/views/errors/auth_exception.xml.haml
diff --git a/server/server.rb b/server/server.rb index 6a0f4cc..0c1755e 100644 --- a/server/server.rb +++ b/server/server.rb @@ -68,16 +68,23 @@ end # # Error handlers # -error Deltacloud::Validation::Failure do +def report_error(status, template) @error = request.env['sinatra.error'] - $stdout.flush - response.status = 400 + response.status = status respond_to do |format| - format.xml { haml :"errors/validation_failure", :layout => false } - format.html { haml :"errors/validation_failure" } + format.xml { haml :"errors/#{template}", :layout => false } + format.html { haml :"errors/#{template}" } end end
+error Deltacloud::Validation::Failure do + report_error(400, "validation_failure") +end + +error Deltacloud::AuthException do + report_error(403, "auth_exception") +end + # Redirect to /api get '/' do redirect '/api'; end
diff --git a/server/views/errors/auth_exception.html.haml b/server/views/errors/auth_exception.html.haml new file mode 100644 index 0000000..1226587 --- /dev/null +++ b/server/views/errors/auth_exception.html.haml @@ -0,0 +1,8 @@ +%h1 Authentication Failure + +%p= @error.message + +%dl + %di + %dt Request URL + %dd= request.env['REQUEST_URI'] diff --git a/server/views/errors/auth_exception.xml.haml b/server/views/errors/auth_exception.xml.haml new file mode 100644 index 0000000..bee6492 --- /dev/null +++ b/server/views/errors/auth_exception.xml.haml @@ -0,0 +1,2 @@ +%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"} + %message #{@error.message}
From: David Lutterkort lutter@redhat.com
* catch_auth was only used in the Rails controllers, but nowhere in the Sinatra version * safely was producing output when it shouldn't; error handling should be left to the toplevel server control --- server/lib/deltacloud/helpers/driver_helper.rb | 19 ------------ server/server.rb | 36 ++++++++++-------------- 2 files changed, 15 insertions(+), 40 deletions(-)
diff --git a/server/lib/deltacloud/helpers/driver_helper.rb b/server/lib/deltacloud/helpers/driver_helper.rb index f1235d5..352253f 100644 --- a/server/lib/deltacloud/helpers/driver_helper.rb +++ b/server/lib/deltacloud/helpers/driver_helper.rb @@ -26,23 +26,4 @@ module DriverHelper Converters::XMLConverter.new( self, type ).convert(obj) end end - - def catch_auth - begin - yield - rescue Deltacloud::AuthException => e - authenticate_or_request_with_http_basic() do |n,p| - end - end - end - - def safely(&block) - begin - block.call - rescue Deltacloud::AuthException => e - @response.status=403 - "<error>#{e.message}</error>" - end - end - end diff --git a/server/server.rb b/server/server.rb index 0c1755e..675658e 100644 --- a/server/server.rb +++ b/server/server.rb @@ -42,25 +42,21 @@ def filter_all(model) filter.merge!(:owner_id => params[:owner_id]) if params[:owner_id] filter.merge!(:state => params[:state]) if params[:state] filter = nil if filter.keys.size.eql?(0) - safely do - singular = model.to_s.singularize.to_sym - @elements = driver.send(model.to_sym, credentials, filter) - instance_variable_set(:"@#{model}", @elements) - respond_to do |format| - format.xml { return convert_to_xml(singular, @elements) } - format.html { haml :"#{model}/index" } - end + singular = model.to_s.singularize.to_sym + @elements = driver.send(model.to_sym, credentials, filter) + instance_variable_set(:"@#{model}", @elements) + respond_to do |format| + format.xml { return convert_to_xml(singular, @elements) } + format.html { haml :"#{model}/index" } end end
def show(model) - safely do - @element = driver.send(model, credentials, { :id => params[:id]} ) - instance_variable_set("@#{model}", @element) - respond_to do |format| - format.xml { return convert_to_xml(model, @element) } - format.html { haml :"#{model.to_s.pluralize}/show" } - end + @element = driver.send(model, credentials, { :id => params[:id]} ) + instance_variable_set("@#{model}", @element) + respond_to do |format| + format.xml { return convert_to_xml(model, @element) } + format.html { haml :"#{model.to_s.pluralize}/show" } end end
@@ -209,12 +205,10 @@ get "/api/instances/new" do end
def instance_action(name) - safely do - @instance = driver.send(:"#{name}_instance", credentials, params[:id]) - respond_to do |format| - format.xml { return convert_to_xml(:instance, @instance) } - format.html { haml :"instances/show" } - end + @instance = driver.send(:"#{name}_instance", credentials, params[:id]) + respond_to do |format| + format.xml { return convert_to_xml(:instance, @instance) } + format.html { haml :"instances/show" } end end
From: David Lutterkort lutter@redhat.com
--- server/lib/deltacloud/base_driver/base_driver.rb | 10 ++++++++++ server/server.rb | 4 ++++ server/views/errors/backend_error.html.haml | 17 +++++++++++++++++ server/views/errors/backend_error.xml.haml | 8 ++++++++ 4 files changed, 39 insertions(+), 0 deletions(-) create mode 100644 server/views/errors/backend_error.html.haml create mode 100644 server/views/errors/backend_error.xml.haml
diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb index eab64a8..04256a4 100644 --- a/server/lib/deltacloud/base_driver/base_driver.rb +++ b/server/lib/deltacloud/base_driver/base_driver.rb @@ -20,6 +20,16 @@ module Deltacloud class AuthException < Exception end
+ class BackendError < StandardError + attr_reader :code, :cause, :details + def initialize(code, cause, message, details) + super(message) + @code = code + @cause = cause + @details = details + end + end + class BaseDriver
def self.define_hardware_profile(name,&block) diff --git a/server/server.rb b/server/server.rb index 675658e..3e5ee07 100644 --- a/server/server.rb +++ b/server/server.rb @@ -81,6 +81,10 @@ error Deltacloud::AuthException do report_error(403, "auth_exception") end
+error Deltacloud::BackendError do + report_error(500, "backend_error") +end + # Redirect to /api get '/' do redirect '/api'; end
diff --git a/server/views/errors/backend_error.html.haml b/server/views/errors/backend_error.html.haml new file mode 100644 index 0000000..a65ff34 --- /dev/null +++ b/server/views/errors/backend_error.html.haml @@ -0,0 +1,17 @@ +%h1 Backend Error + +%p= @error.message + +%dl + %di + %dt Request URL + %dd= request.env['REQUEST_URI'] + %di + %dt Code + %dd= @error.code + %di + %dt Cause + %dd= @error.cause + %di + %dt Details + %dd= @error.details diff --git a/server/views/errors/backend_error.xml.haml b/server/views/errors/backend_error.xml.haml new file mode 100644 index 0000000..75866eb --- /dev/null +++ b/server/views/errors/backend_error.xml.haml @@ -0,0 +1,8 @@ +%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"} + %kind backend_error + %backend{ :driver => DRIVER } + %code= @error.code + %cause= @error.cause + - if @error.details + %details #{@error.details} + %message #{@error.message}
From: David Lutterkort lutter@redhat.com
--- .../drivers/rackspace/rackspace_driver.rb | 2 +- .../lib/deltacloud/drivers/rhevm/rhevm_driver.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb index 051d44a..d1b7f4e 100644 --- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb @@ -130,7 +130,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
def new_client(credentials) - RackspaceClient.new(credentials.name, credentials.password) + RackspaceClient.new(credentials.user, credentials.password) end
define_instance_states do diff --git a/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb b/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb index 32c4f90..e40558d 100644 --- a/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb +++ b/server/lib/deltacloud/drivers/rhevm/rhevm_driver.rb @@ -61,7 +61,7 @@ class RHEVMDriver < Deltacloud::BaseDriver end
def genArgString(credentials, args) - commonArgs = [SCRIPT_DIR_ARG, credentials.name, credentials.password, CONFIG["domain"]] + commonArgs = [SCRIPT_DIR_ARG, credentials.user, credentials.password, CONFIG["domain"]] commonArgs.concat(args) commonArgs.join(" ") end
From: David Lutterkort lutter@redhat.com
--- server/bin/deltacloudd | 34 +++++++++++++++++++++++++++------- 1 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/server/bin/deltacloudd b/server/bin/deltacloudd index 0d0bce0..a1c8f09 100755 --- a/server/bin/deltacloudd +++ b/server/bin/deltacloudd @@ -57,12 +57,32 @@ argv_opts << ['--threaded', '-D', '--stats', '/stats']
argv_opts.flatten!
-thin = Thin::Runner.new(argv_opts) +if options[:env] == "development" + use_rerun = false + begin + require "rerun" + use_rerun = true + rescue + # Do nothing + end +end + +puts "Starting Deltacloud API :: #{ENV["API_DRIVER"]} :: http://#%7BENV%5B%22API_HOST%22%5D%7D:#%7BENV%5B%22API_PORT%22%5D%7D/api" +puts
-begin - puts "Starting Deltacloud API :: #{ENV["API_DRIVER"]} :: http://#%7BENV%5B%22API_HOST%22%5D%7D:#%7BENV%5B%22API_PORT%22%5D%7D/api" - puts - thin.run! -rescue Exception => e - puts "ERROR: #{e.message}" +if use_rerun + argv_opts.unshift "thin" + command = argv_opts.join(" ") + topdir = File::expand_path(File::join(File::dirname(__FILE__), "..")) + rerun = Rerun::Runner.new(command, :dir => topdir) + rerun.start + rerun.join +else + thin = Thin::Runner.new(argv_opts) + + begin + thin.run! + rescue Exception => e + puts "ERROR: #{e.message}" + end end
From: David Lutterkort lutter@redhat.com
* raise AuthException when we can't authenticate * propagate all other non-success messages from talking to Rackspace as BackendError --- .../drivers/rackspace/rackspace_client.rb | 57 ++++++++++++++++---- 1 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_client.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_client.rb index d01d8d3..6503d51 100644 --- a/server/lib/deltacloud/drivers/rackspace/rackspace_client.rb +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_client.rb @@ -36,16 +36,17 @@ class RackspaceClient http = Net::HTTP.new(@@AUTH_API.host,@@AUTH_API.port) http.use_ssl = true authed = http.get(@@AUTH_API.path, {'X-Auth-User' => username, 'X-Auth-Key' => auth_key}) + if authed.is_a?(Net::HTTPUnauthorized) + raise Deltacloud::AuthException, "Failed to authenticate to Rackspace" + elsif !authed.is_a?(Net::HTTPSuccess) + backend_error!(resp) + end @auth_token = authed.header['X-Auth-Token'] @service_uri = URI.parse(authed.header['X-Server-Management-Url']) @service = Net::HTTP.new(@service_uri.host, @service_uri.port) @service.use_ssl = true end
- def get(path) - @service.get(@service_uri.path + path, {"Accept" => "application/json", "X-Auth-Token" => @auth_token}).body - end - def list_flavors JSON.parse(get('/flavors/detail'))['flavors'] end @@ -65,17 +66,22 @@ class RackspaceClient
def start_server(image_id, flavor_id, name) - json = { :server => { :name => name, :imageId => image_id, :flavorId => flavor_id }}.to_json - JSON.parse(@service.post(@service_uri.path + "/servers", json, headers).body) + json = { :server => { :name => name, + :imageId => image_id.to_i, + :flavorId => flavor_id.to_i }}.to_json + # FIXME: The response has the root password in 'adminPass'; we somehow + # need to communicate this back since it's the only place where we can + # get it from + JSON.parse(post("/servers", json, headers).body)["server"] end
def delete_server(server_id) - @service.delete(@service_uri.path + "/servers/#{server_id}", headers) + delete("/servers/#{server_id}", headers) end
def reboot_server(server_id) json = { :reboot => { :type => :SOFT }}.to_json - @service.post(@service_uri.path + "/servers/#{server_id}/action", json, headers) + post("/servers/#{server_id}/action", json, headers) end
@@ -83,12 +89,41 @@ class RackspaceClient {"Accept" => "application/json", "X-Auth-Token" => @auth_token, "Content-Type" => "application/json"} end
-end - + private + def get(path) + resp = @service.get(@service_uri.path + path, {"Accept" => "application/json", "X-Auth-Token" => @auth_token}) + unless resp.is_a?(Net::HTTPSuccess) + backend_error!(resp) end + resp.body end -end
+ def post(path, json, headers) + resp = @service.post(@service_uri.path + path, json, headers) + unless resp.is_a?(Net::HTTPSuccess) + backend_error!(resp) + end + resp + end
+ def delete(path, headers) + resp = @service.delete(@service_uri.path + path, headers) + unless resp.is_a?(Net::HTTPSuccess) + backend_error!(resp) + end + resp + end
+ def backend_error!(resp) + json = JSON.parse(resp.body) + cause = json.keys[0] + code = json[cause]["code"] + message = json[cause]["message"] + details = json[cause]["details"] + raise Deltacloud::BackendError.new(code, cause, message, details) + end
+end + end + end +end
From: David Lutterkort lutter@redhat.com
In particular, * do not expect srv["addresses"] to be set * turn various ints into strings --- .../drivers/rackspace/rackspace_driver.rb | 27 ++++++++++--------- 1 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb index d1b7f4e..2dd34b8 100644 --- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb @@ -114,21 +114,22 @@ class RackspaceDriver < Deltacloud::BaseDriver
def convert_srv_to_instance(srv) - Instance.new( { - :id=>srv["id"], - :state=>srv["status"] == "ACTIVE" ? "RUNNING" : "PENDING", - :name=>srv["name"], - :image_id=>srv["imageId"], - :owner_id=>"root", - :realm_id=>"us", - :public_addresses=>( srv["addresses"]["public"] ), - :private_addresses=>( srv["addresses"]["private"] ), - :flavor_id=>srv["flavorId"], - :actions=>instance_actions_for(srv["status"] == "ACTIVE" ? "RUNNING" : "PENDING"), - } ) + status = srv["status"] == "ACTIVE" ? "RUNNING" : "PENDING" + inst = Instance.new(:id => srv["id"].to_s, + :owner_id => "root", + :realm_id => "us") + inst.name = srv["name"] + inst.state = srv["status"] == "ACTIVE" ? "RUNNING" : "PENDING" + inst.actions = instance_actions_for(inst.state) + inst.image_id = srv["imageId"].to_s + inst.flavor_id = srv["flavorId"].to_s + if srv["addresses"] + inst.public_addresses = srv["addresses"]["public"] + inst.private_addresses = srv["addresses"]["private"] + end + inst end
- def new_client(credentials) RackspaceClient.new(credentials.user, credentials.password) end
On 10/03/10 21:22 -0800, lutter@redhat.com wrote:
These patches started with me tryingto fix whatever was going wrong in the Rackspace driver. In the process I improved the infrastructure for reporting errors from talking to a backend cloud into the XML and HTML responses.
ACKing all changes, only few questions:
Doesn't thin handle code reloading when it's running in development mode ?
I suggest to decide, which framework we will use for manipulating with XMLs in drivers, which isn't supported by Rightscale gems. I see we are using 'NET:HTTP' in rackspace_client.rb (and some other clients'). I'm voting for RestClient/nokogiri here.
- Michal
David _______________________________________________ deltacloud-devel mailing list deltacloud-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/deltacloud-devel
On Fri, 2010-03-12 at 11:50 +0100, Michal Fojtik wrote:
Doesn't thin handle code reloading when it's running in development mode ?
It didn't for me - did that work for you ?
I suggest to decide, which framework we will use for manipulating with XMLs in drivers, which isn't supported by Rightscale gems. I see we are using 'NET:HTTP' in rackspace_client.rb (and some other clients'). I'm voting for RestClient/nokogiri here.
Yes, absolutely agree - I just wanted to fix the most important problems with these patches first.
David
deltacloud-devel@lists.fedorahosted.org